ome / omero-parade

OMERO.web plugin for displaying Dataset thumbnails or Plates in webclient center panel
GNU Affero General Public License v3.0
1 stars 12 forks source link

csv support #75

Closed will-moore closed 4 years ago

will-moore commented 4 years ago

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

To test:

emilroz commented 4 years ago

@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 commented 4 years ago

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

pwalczysko commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@pwalczysko You reported this already in 2018: https://github.com/ome/omero-parade/issues/43

pwalczysko commented 4 years ago

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

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Couple of comments. Maybe some of them are generally about parade. I think https://github.com/ome/omero-parade/pull/75#issuecomment-588901886 is pertaining to this workflow though

pwalczysko commented 4 years ago

And https://github.com/ome/omero-parade/pull/75#issuecomment-588171135 is fixed.

pwalczysko commented 4 years ago

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 commented 4 years ago

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.

jburel commented 4 years ago

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

will-moore commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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