search

broadinstitute / single_cell_portal_core

Rails/Docker application for the Broad Institute's single cell RNA-seq data portal
https://singlecell.broadinstitute.org
BSD 3-Clause "New" or "Revised" License
62 stars 26 forks source link

Handle missing cluster-based annotation in plot data cache (SCP-5799) #2128

Closed bistline closed 1 month ago

bistline commented 1 month ago

BACKGROUND & CHANGES

This fixes a bug where a missing cluster-based annotation in spatial plots causes all expression-based scatter plots to throw an error. This was due to the scatter object having a value of undefined for annotParams in the response when trying to populate the cache. Now, the plot correctly shows the warning that the requested annotation does not exist on that plot.

Example:

Screenshot 2024-09-16 at 11 08 27 AM

This update also deals with some test instability issues in ingest_job_test.rb and import_service_config/nemo_test.rb due to recent updates, order of operations, etc.

MANUAL TESTING

  1. Boot all services and sign in
  2. If you do not have a copy of the chick_raw_counts synthetic study in your instance, you can load it from the Rails console with: 
    SyntheticStudyPopulator.populate('chicken_raw_counts')
  3. If you do have a copy, go to the upload wizard for that study and upload db/seed/synthetic_studies/chicken_raw_counts/cluster_odd_labels.txt file as a normal cluster file
  4. Once ingested, go to the Explore tab for this study and select cluster_odd_labels, and then odd_labels as the annotation
  5. Load the spatial cluster spatial_letters to confirm you see the "spatial_letters" does not have the requested annotation "odd_labels--group--cluster" warning
  6. Search for the gene RAD51 and confirm you see the same warning in the spatial_letters expression plot
codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 69.72%. Comparing base (b7dd9bb) to head (90d4c34). Report is 8 commits behind head on development.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2128/graphs/tree.svg?width=650&height=150&src=pr&token=HMWE5BO2a4&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute)](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2128?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute) ```diff @@ Coverage Diff @@ ## development #2128 +/- ## ============================================ Coverage 69.72% 69.72% ============================================ Files 328 328 Lines 27644 27649 +5 Branches 2298 2395 +97 ============================================ + Hits 19274 19279 +5 Misses 8236 8236 Partials 134 134 ``` | [Files with missing lines](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2128?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute) | Coverage Δ | | |---|---|---| | [...p/javascript/components/explore/plot-data-cache.js](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2128?src=pr&el=tree&filepath=app%2Fjavascript%2Fcomponents%2Fexplore%2Fplot-data-cache.js&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL2phdmFzY3JpcHQvY29tcG9uZW50cy9leHBsb3JlL3Bsb3QtZGF0YS1jYWNoZS5qcw==) | `92.46% <100.00%> (ø)` | | | [...avascript/components/visualization/ScatterPlot.jsx](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2128?src=pr&el=tree&filepath=app%2Fjavascript%2Fcomponents%2Fvisualization%2FScatterPlot.jsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL2phdmFzY3JpcHQvY29tcG9uZW50cy92aXN1YWxpemF0aW9uL1NjYXR0ZXJQbG90LmpzeA==) | `64.20% <ø> (-0.17%)` | :arrow_down: | | [app/models/import\_service\_config/nemo.rb](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2128?src=pr&el=tree&filepath=app%2Fmodels%2Fimport_service_config%2Fnemo.rb&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL21vZGVscy9pbXBvcnRfc2VydmljZV9jb25maWcvbmVtby5yYg==) | `55.88% <100.00%> (ø)` | | ... and [6 files with indirect coverage changes](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2128/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute)
bistline commented 1 month ago

Non-blocking suggestion: allow the plots that lack the requested annotations to continue to have their plot titles rendered. This gives context to the warning about lacking annotations. Otherwise the warning floats "in space" and we lose track of what data is missing the annotation.

90d4c34

I will note that the plot title is preserved in the warning message, but it's not very visible, so this suggestion is much more coherent.