Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

csv support #75

Merged
merged 13 commits into from
Feb 27, 2020
Merged

csv support #75

merged 13 commits into from
Feb 27, 2020

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Feb 10, 2020

This adds CSV support to OMERO.parade for filtering and table_columns / scatter plot.

To test:

  • Attach CSV to Project or Plate. (NB: not tested on Dataset yet). Should have an Image column or image_id column (NB: this can be created with Batch_ROI_Export.py but may give confusing results if there is more than 1 row per Image ID.
  • In parade, the Filter drop-down list should list each .csv file on the Project/Plate. NB: only alpha-numeric characters, underscore and . are allowed in this list since parade UI uses the chosen option to the URL directly. Other characters are currently replaced by underscore. Alternative option is to relax the URL regex to allow more flexibility. Currently this is r'^filters/script/(?P<filter_name>[\w.]+)/$'
  • Choose a .csv filter, the filter should load as for OMERO.tables. NB: Only numerical columns are used - any columns with non-numbers in will be ignored.
  • Choose a column should allow filtering by number as for Tables data
  • Adding data to the table layout and scatter plot: For each csv file, all the columns are listed in the format file.csv column in the drop-down list. This allows choosing columns from multiple different CSV files.
  • Choosing column should load the data as for OMERO.tables, add the column to the table and allow plotting in the scatter plot.
  • Test this workflow for Project and Plate.
  • NB: the Plate at https://merge-ci.openmicroscopy.org/web/webclient/?show=plate-13855 has 1 ROI per image and you can use the Batch_ROI_Export.py to create a CSV since Batch export ROIs from screen omero-scripts#156 adds Plate support.
  • To test the CSV file size limit I've set the temp limit at 1MB at https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-web/ with
    omero config set omero.web.parade.max_csv_size 1000000. Any CSV file over this size should be ignored by filters and 'table data'. The default limit without this setting is 100MB.

@will-moore will-moore mentioned this pull request Feb 10, 2020
@will-moore will-moore changed the title Add csv_filters module for CSV filter and data csv support Feb 10, 2020
@emilroz
Copy link
Member

emilroz commented Feb 11, 2020

@will-moore would it be possible to impose a file size limit? We often deal with CSVs in 10s of GBs which could potentially lead to pretty severe side effects.

@will-moore
Copy link
Member Author

@emilroz Done (see last commit). I've set the default to 100MB and this is configurable.

@pwalczysko
Copy link
Member

pwalczysko commented Feb 19, 2020

Created a 13 ROIs-strong csv using the Batch ROI export script on a Project.

See https://merge-ci.openmicroscopy.org/web/webclient/?show=project-15107
Login as user-3
But when the dropdown menu above cetral pane "Parade" is chosen, then in the top menu "...csv" is selected, I get an empty dialog with Error - Send feedback.

@pwalczysko
Copy link
Member

Actually, the problem is not that the whole app does not work. I can still use the lower menu, which adds columns, and it works fine.

@pwalczysko
Copy link
Member

pwalczysko commented Feb 19, 2020

Screen Shot 2020-02-19 at 11 37 25
the ticks are not there in FF - see above - the values in the graph are 8, 15 and 51

edit: This is a firefox-specific issue - works fine in Chrome

@will-moore
Copy link
Member Author

@pwalczysko You reported this already in 2018: #43

@pwalczysko
Copy link
Member

Filtering for "Image" from the csv does not make sense and/or does not work:

  • have only one csv on Project https://merge-ci.openmicroscopy.org/web/webclient/?show=project-15107 (user-3)
  • the first two columns of the csv are image_id and image_name (the csv was created by Batch ROI export script)
  • select in parade the filtering according to that csv
  • in a following menu, select the first item Image
  • observe that the filtering either does not work or does not make sense (is it image id I am filtering for here ? Where are the sugestinons of available range in the tooltip ? whatever I put into the box, nothing gets filtered, why ?)

P.S. After trying to put some numbers in (not text as previously), I can see that the filtering is happening, but still, no tooltip suggestions, and where is the id from the name of the filter ? Image is not sufficient to understand this filter.

@pwalczysko
Copy link
Member

Screen Shot 2020-02-20 at 10 24 45
As I have only one value of t in my csv, namely t=1, the histogram looks very funny (see above) and the slider is there - long and with the button, but it "does not work" - the UI in this case passes a wrong impression - maybe in such case (one value only) the histogram and the slider should not be there ?

@pwalczysko
Copy link
Member

Screen Shot 2020-02-20 at 10 42 51

The channel column produces another illogical behaviour. Probably caused by my csv having in one case an image with 3 rows (3 channel image). The slider works, and it kind of makes sense, but only and solely because I know how the parade expects one row per image, how it takes randomly (I can see in this case rather that it takes the first value) the row for that image. If I do not know that, I would be again confused.

@pwalczysko
Copy link
Member

Area is another interesting case. As some of my images have ROIs which have no areas, they just vanish when this filter comes up. This has some justification. Nevertheless, it invites the question about the empty values in my csv, which is much more general. Does no value mean "filter me out of sight" ? Maybe better would be a warning+some other highlighting of the images (greying out ?) as the N/A ones

@pwalczysko
Copy link
Member

The lenght filtering (2 valid values only) does not seem to make sense.
Even if I use the = sign for filtering, I will not see my 2 images ever. Rounding errors ?

@pwalczysko
Copy link
Member

Couple of comments. Maybe some of them are generally about parade. I think #75 (comment) is pertaining to this workflow though

@pwalczysko
Copy link
Member

And #75 (comment) is fixed.

@pwalczysko
Copy link
Member

I think #75 (comment) is pertaining to this workflow though

Discussed with @will-moore : the image_id -> Image mapping from the csv column header to the parade is a fixed feature of parade. Probably hard to improve that without starting a new re-design of parade.

Happy to merge then, as the PR still brings an important new functionality

@will-moore
Copy link
Member Author

Thanks @pwalczysko - Yes, I don't want to try and fix all the UI issues in this PR.
We should probably update the Batch_Image_Export.py script to use Image as a column name for image ID, as we do for tables.

omero_parade/csv_filters/data_providers.py Outdated Show resolved Hide resolved
MAX_CSV_SIZE = parade_settings.MAX_CSV_SIZE


def get_csv_annotations(conn, obj_type, obj_id):
Copy link
Member

Choose a reason for hiding this comment

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

another example of method that should go to the gateway

@jburel
Copy link
Member

jburel commented Feb 21, 2020

Overall it looks fine but we should now start thinking of methods that we can re-use since we do the same over and over either in training example script/analysis examples or web app.
It fixes a problem but does not allow re-usability for developers willing to use the API

omero_parade/csv_filters/data_providers.py Outdated Show resolved Hide resolved
omero_parade/csv_filters/data_providers.py Outdated Show resolved Hide resolved
omero_parade/csv_filters/omero_filters.py Outdated Show resolved Hide resolved
omero_parade/csv_filters/omero_filters.py Outdated Show resolved Hide resolved
@will-moore
Copy link
Member Author

I redeployed that last commit and added a CSV file to https://merge-ci.openmicroscopy.org/web/webclient/?show=project-6358 user-3 with an image column.
This is working now for filtering, table data and plotting.

@jburel
Copy link
Member

jburel commented Feb 27, 2020

idr0021 does not have in the list of filter Batch_ROI_Export.csv

edit: discussed with @will-moore This is due to the fact that there is a limit set for large CSV.
so the limit is working :-)

@will-moore
Copy link
Member Author

Yes, OMERO-web job had omero config set omero.web.parade.max_csv_size 1000000 and the Batch_ROI_Export.csv was over 1MB. Redeployed now without that setting and the csv is available.

@jburel
Copy link
Member

jburel commented Feb 27, 2020

The file is now available and working.
Merging and tagging a new version of parade

@jburel jburel merged commit 4bd66d1 into ome:master Feb 27, 2020
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.

4 participants