Skip to content

[WIP] CSV datatype#191

Merged
jmchilton merged 6 commits intogalaxyproject:devfrom
vavrusa:table-datatype
Jun 4, 2015
Merged

[WIP] CSV datatype#191
jmchilton merged 6 commits intogalaxyproject:devfrom
vavrusa:table-datatype

Conversation

@vavrusa
Copy link
Copy Markdown
Contributor

@vavrusa vavrusa commented Apr 30, 2015

This is another attempt at the CSV type support, I'd like to know your thoughts before I move forward with it.

Fixes

  • made a superclass out for Tabular (TabularData) with metadata and generic methods
  • made a CSV class for CSV-style tabular data
  • stats/gsummary works with RPy2 classic mode, as the original RPy is pretty much dead

How to test

I have attached some example files at https://gist.github.com/vavrusa/f8bb8d059c6c96dc7585
to test it, the one with the first line as a header should be sniffed correctly when uploading, the rest should be correctly displayed when saved as tabular. Visualization for all should be correct.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this seems pretty solid to me - can you create a tabular file on master (which won't have the delimiter metadata defined), then checkout this branch and verify these column providers still work with the old datasets. I think there are other places we are defensive for older datasets and newer metadata - I just wanted to make sure you have confirmed that isn't needed here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chatted with @blankenberg about this - it seems that this is what no_value= does in the MetadataElement definition about. So I guess the no_value for delimiter should be \t.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, I just checked - to me it looks okay (file: csv-tab-commentheader-csv)

@jmchilton
Copy link
Copy Markdown
Member

Yeah - I just check it and somehow it is getting '\t' as the delimiter even though the no_value=[]. Still think the no_value="\t" would be a better default though for that metadata element.

So does the scatterplot work for you? I don't get any visualizations enabled except IPython if I upload a csv file.

@jmchilton
Copy link
Copy Markdown
Member

Haven't heard any news on this for a couple weeks - any chance for status update? I am going to merge #204 soon unless there is a strong objection which will introduce more conflicts to this.

@vavrusa
Copy link
Copy Markdown
Contributor Author

vavrusa commented May 22, 2015

Yeah, the notification slipped my mind. The scatterplots works for me, maybe it's not in your configuration? The thing is, that everything in config/ is blocked by https://github.com/galaxyproject/galaxy/blob/dev/.gitignore#L65 (which is a bug btw.), even the config/plugins/visualizations/charts/config/charts.xml, so maybe it didn't update for you?

dannon and others added 3 commits June 2, 2015 18:45
Conflicts:
	lib/galaxy/datatypes/tabular.py
For me it seems like the viz plugins only detect the datatype if the plugin datatype (e.g. ``tabular.CSV``) is explicitly instantiated by some extension directly. In your sample config you have ``subclass="True"`` so ``tabular.CSV`` wasn't working. This is probably a bug with the datatype code - but nonetheless I think dropping subclass from the sample is an okay workaround - because it does seem to be a direct mapping.
@jmchilton
Copy link
Copy Markdown
Member

Figured out the viz problem - I think a small tweak to the sample config fixes it. I have opened a PR against your branch that resolves this and merges in the latest dev so the PR reapplies cleanly.

jmchilton added a commit that referenced this pull request Jun 4, 2015
@jmchilton jmchilton merged commit 0dfa1c9 into galaxyproject:dev Jun 4, 2015
@martenson
Copy link
Copy Markdown
Member

Thank you for the contribution @vavrusa and @jmchilton for the review.

@vavrusa
Copy link
Copy Markdown
Contributor Author

vavrusa commented Jun 4, 2015

🎉 thanks!

lldelisle pushed a commit to lldelisle/galaxy that referenced this pull request Oct 13, 2023
* update outputs

* add default notebook

* add an inputs folder
domgz pushed a commit to domgz/galaxyproject-galaxy that referenced this pull request Oct 30, 2023
* update outputs

* add default notebook

* add an inputs folder
sebastian-schaaf pushed a commit to sebastian-schaaf/galaxy that referenced this pull request Nov 16, 2023
* update outputs

* add default notebook

* add an inputs folder
sebastian-schaaf pushed a commit to sebastian-schaaf/galaxy that referenced this pull request Nov 20, 2023
* update outputs

* add default notebook

* add an inputs folder
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants