ome / omero-gallery

https://pypi.org/project/omero-gallery/
GNU Affero General Public License v3.0
5 stars 15 forks source link

Idr prototype #25

Closed will-moore closed 5 years ago

will-moore commented 5 years ago

Update to Gallery to allow searching & filtering of top-level Projects & Screens and a nicer layout showing 'categories' of interest, selected by querying Map-Annotations. See https://github.com/openmicroscopy/design/issues/100

Can be tested at http://web-dev-merge.openmicroscopy.org/gallery/

For screenshots etc see the design issue link above.

TODO:

Need Help:

will-moore commented 5 years ago

Summary of Study Types: count (NB: duplicate studies e.g. idr0038 are counted multiple times).

3D-tracking of tagged chromatin loci: 1
4D timelapse of in vivo cells: 1
5D light sheet and confocal microscopy imaging of mouse zygotes: 1
Confocal laser scanning microscopy of cytoskeletal systems: 1
Quantitative protein localization using FCS-calibrated 3D time-lapse imaging: 1
high content analysis of cells: 1
high content screen: 37
high resolution electron microscopy: 1
histology: 2
imaging mass cytometry: 1
in situ hybridization: 1
light sheet fluorescence microscopy of TARDIS registered embryos: 1
light sheet fluorescence microscopy of organoids: 4
light sheet microscopy of zebrafish tailbud: 1
microscopy assay: 4
protein localization: 1
protein localization using 3D-SIM: 1
protein localization using dSTORM: 1
timelapse microscopy: 1
will-moore commented 5 years ago

Some potential categories: IDs are idr numbers.

"time-lapse": [41, 27, 26, 40],
"lightsheet": [51, 45,  44, 38],
"protein localization": [43, 41, 23, 21, 19],
"histology": [42, 18, 43],
"yeast": [3, 4, 7, 11, 27, 40, 47],
"human nuclear stain": [16, 9, 6, 17, 30, 36, 10]
manics commented 5 years ago

If I enter a search term which returns some results, then delete the search term, the results are removed but the count Showing 37 of 63 studies remains unchanged:

Screen Shot 2019-05-03 at 16 47 19
francesw commented 5 years ago

Page looks good!

Here are some comments.

Heading: Welcome to the Image Data Resource (IDR).
Remove “Cells” and “Tissue” from images and have “Cell-IDR” and “Tissue-IDR” above images?

Drop-down for search/filter: • Remove “Study Attributes” and “Image Attributes”? Note: Organism appears twice.
• Search/filter list (in alphabetical order?) = IDR Accession (default?), Imaging Method, Organism, Cell Line, Gene, siRNA, Antibody, Compound, Phenotype, Publication Author, Publication Title.
• For publication authors, include last authors as well (i.e. the PI).
• When typing in search box, list of suggestions should reflect what has been typed in the search box (i.e. when “t” is in Imaging Method and “imaging mass cytometry” should not be at the top of the list).
• Possibly not have the space/gap between the filter and search box?

Question: • Is it possible to type in a search term without selecting a filter?

Categories suggestions: Time-lapse imaging, Light sheet fluorescence microscopy, High-content screening, Digital pathology imaging (instead of histology?), Protein localization studies, Human cell line studies, Yeast studies.

• Make scroll bar more obvious for categories with more than 4 results?
• Category links should be directed to new page containing the thumbnails for that category (i.e. light sheet = 7)?
• Only show categories with results for cell and tissue IDR pages.

manics commented 5 years ago

When the autocomplete dropdown contains long lines (e.g. study titles) the wrapping means the second line is indented to the left of the first:

Screen Shot 2019-05-03 at 17 00 30

Is it possible to get the second line to be indented to the right instead by several spaces, I think this would be easier to read. maybe a combination of padding-left: NN; text-indent: -NN;?

manics commented 5 years ago

Suggestion: Add idr accession number as an option, allow entries such as idr0052 0052, 52

will-moore commented 5 years ago

New to test today:

manics commented 5 years ago

Suggestion: have a link to go back to the default view of everything

Minor UI bug (not a blocker for merging): light sheet fluorescence microscopy of TARDIS registered embryos flips between between split over one line and two depending on whether it's highlighted:

Screen Shot 2019-05-07 at 11 05 54 Screen Shot 2019-05-07 at 11 06 01

The lack of default suggestions for the Image Attributes is unexpected after the Study Attributes, but if this is going too involve some mapr or caching optimisation this can go into a new issue.

joshmoore commented 5 years ago

@will-moore : would it be possible to migrate the bigger items from your TODO list to separate issues? For discussions at this point, we'd have to reference the line from your description and then comment. It will make discussions hard to follow. From my side, I'm for getting this PR in as quickly as possible, and then following up with other targeted PRs. If you're worried about having conflicts with yourself, then perhaps we do that via milestones (this being M1?) rather than a PR per TODO. So if you had 5 issues for a given milestone, then in the description you could have:

 - [x] fix #A
 - [x] fix #B

and merging that new PR would close all the tickets.

will-moore commented 5 years ago

Request to filter by License https://lists.openmicroscopy.org.uk/mailman/private/idr-submission/2019-May/002307.html added in e1c3bcd

chris-allan commented 5 years ago

Chrome on Windows 10. Just over 1080p browser window size. Default font scaling.

Few pieces of feedback for the landing page:

  1. Getting a series of JavaScript errors in the console on initial page load:
    model.js:312 Uncaught (in promise) TypeError: Cannot read property 'json' of undefined
    at model.js:312
    (anonymous) @ model.js:312
    Promise.then (async)
    loadImage @ model.js:313
    (anonymous) @ categories.js:223
    loadStudyThumbnails @ categories.js:217
    render @ categories.js:173
    (anonymous) @ categories.js:264
    (anonymous) @ model.js:191
    Promise.then (async)
    loadStudiesMapAnnotations @ model.js:162
    (anonymous) @ model.js:144
    Promise.then (async)
    loadStudies @ model.js:124
    (anonymous) @ categories.js:259
    model.js:305 No Dataset in Project! {meta: {…}, data: Array(0)}
    model.js:312 Uncaught (in promise) TypeError: Cannot read property 'json' of undefined
    at model.js:312
  2. I've got a pretty fast connection, am ~20ms from idr.openmicroscopy.org and the page takes over 10 seconds to fully load. Might be an idea to batch thumbnail calls via get_thumbnails. The page makes 50+ individual requests for thumbnails at the moment and this will only get worse over time.
  3. Not confining the author lists and not having some line breaks in the descriptions creates some pretty wacky visualization conditions: image image
  4. The scaling of the thumbnail strips is creating some weird excessive whitespace in the horizontal direction. You can scroll well beyond any thumbnails: image
  5. Yeast Studies scrollbar/header overlap: image

And for search:

  1. If idr0018 is matched by a search, such as "microscopy", it is missing some of its metadata: image
  2. Feels kind of weird to have a search results grid that is only two studies wide image
will-moore commented 5 years ago

@chris-allan Thanks for the feedback. I've fixed the console errors and search results is now 3 columns wide. The other css fixes I have put on hold since the whole page is likely to get a facelift and I was already spending too much time trying to design and fix those.

chris-allan commented 5 years ago

:+1:

Already looking pretty solid and adept at what it's trying to do. Is your github.io up to date with the latest code? Just noticed there are still console errors there.

will-moore commented 5 years ago

@sbesson @francesw As noted by @chris-allan, idr0018 is missing Publication Title both in the Project Description and in the linked Map Annotation. I'm getting this from the Description to show in the UI like this:

  let titleRe = /Publication Title\n(.+)[\n]?/
  let match = titleRe.exec(desc);
  let title = match ? match[1] : '';

Do we want to add some title to this study to "fix" the UI?

will-moore commented 5 years ago

@chris-allan I hadn't pushed the console error fixes to github.io. But did it just now.

chris-allan commented 5 years ago

Great. Definitely no error messages now.

will-moore commented 5 years ago

To test last commit (batch loading of thumbnails), first login to "idr" as "public/pubilc" at http://web-dev-merge.openmicroscopy.org/webclient/login/ then go to: http://web-dev-merge.openmicroscopy.org/gallery/ You should see thumbnails loading much faster than before. Instead of making 2 JSON calls to get the image ID then the loading of the thumb itself (3 calls for each image) we now load the Image IDs and thumbnails for 10 studies/thumbnails at once. (30 x fewer calls). URLs like http://web-dev-merge.openmicroscopy.org/gallery/api/thumbnails/?project=51&project=101&project=402&screen=1603&screen=1602&screen=1601&screen=1551&screen=1501&screen=201&screen=202 are used to get thumbnails AND image IDs (needed for the viewer links) in one shot.

chris-allan commented 5 years ago

Looking now at the aggregate changes in urls.py and views.py I can see us slowly going down the road of adding some pretty fundamental API to an OMERO.web plugin. Representative thumbnails for containers is something that has been in the background for a long time. Glencoe did something similar for the JCB DataViewer in what we called the “featured” image via a specifically namespaced BooleanAnnotation and a fallback to the first image if missing.

I’d like to propose that we iterate on this API using https://github.com/glencoesoftware/omero-ms-thumbnail. Doing so is very much in the spirit of trying to avoid the deployment idiosyncrasies mentioned above, reinforces a separation of concerns, and also helps avoid the need for a full OMERO.web deployment being required to contribute to this plugin.

/cc @kkoz, @emilroz

sbesson commented 5 years ago

Re-tested the latest version of this code on the deployed CI this morning. Most of the bug fixes discussed above have been addressed. I will capture some errors as issues.

As noted in https://github.com/ome/omero-gallery/pull/25#issuecomment-491765164, the size of this PR is starting to be big enough that reviewing the code and the comments becomes quickly impractical. In line with various conversations, I think we all agree that we have now reached the end of the first phase which was dedicated to building this UI.

Proposing to tag this initial effort as 3.2.0a1 and start working on the next steps in smaller steps, releasing the app in milestones as needed:

  1. make the changes to deploy this application into a testing resource in preparation of production deployment. This might require a couple of additional cleanup PRs to remove hard-coded configuration, enable thumbnail viewing without the need to log in etc.
  2. iterate over the UI issues (CSS & other bugs) as well as the metadata that should be added to the studies to make the UI functional
  3. once deployed, schedule some performance testing of the new UI
  4. investigate the migration of the required thumbnail API calls to a micro-service as suggested in https://github.com/ome/omero-gallery/pull/25#issuecomment-494756166. This might also be highlighted by the load testing.