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

Fixing scatter plot legends in spatial UX (SCP-5626) #2070

Closed bistline closed 3 months ago

bistline commented 4 months ago

BACKGROUND & CHANGES

This update fixes issues discovered with scatter plot legends when viewing multiple plots in the spatial UX. Whenever a user selects a new spatial plot, every legend shown then updates the counts to use data from the newest spatial plot. Additionally, if certain labels are missing from the newest plot, those legend entries are still shown but with a count of 0.

The cause was from a shared useState reference in ExploreDisplayTabs for setting label counts (this was done so it could be shared with various components in the differential expression UX). Now, counts are managed at the ScatterPlot level instead of ExploreDisplayTabs. Additionally, color assignments are now tracked using the main, non-spatial plot as the control. Any additional spatial plots will use that color map to ensure that labels keep the same color assignment across all plots.

Note: you may see a brief flicker as colors are reassigned, this has to do with waiting for the main plot to finish rendering.

MANUAL TESTING

The easiest way to test this PR is to ingest data from SCP1375 (this is the study where the above bugs were found). Only 4 files are needed for testing:

  1. Boot all services and sign in
  2. Create a new study and ingest the 4 files above, making sure to upload the last two cluster files as spatial
  3. Once all clustering/metadata have ingested, go to the Explore tab for this study (click "View study" at the top left of the upload wizard)
  4. Load top_level_umap.csv and top_level_cell_type as the annotation
  5. In the Spatial groups menu, select spatial_8months-control-replicate_1.csv
  6. Confirm the count for Astro is 6789 for top_level_umap.csv and 919 for spatial_8months-control-replicate_1.csv
  7. Confirm that spatial_8months-control-replicate_1.csv does not have the group LHb, and the color for Micro and the remaining labels match in both plots
  8. Add spatial_13months-disease-replicate_2.csv from the spatial groups menu
  9. Confirm the count for Astro is 927 and that it has the LHb group with a count of 65 and the correct color
  10. Change the annotation multiple times (including back to a value you've already selected) and confirm that color assignments match across all plots.
    • If you select Hide all in the legend for the main plot then scroll to the bottom and select the last entry, this is a quick way to ensure that assignments have stayed consistent across all plots.
codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 71.01449% with 20 lines in your changes missing coverage. Please review.

Project coverage is 69.63%. Comparing base (e42082e) to head (a02f384). Report is 3 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/2070/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/2070?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute) ```diff @@ Coverage Diff @@ ## development #2070 +/- ## =============================================== - Coverage 69.64% 69.63% -0.02% =============================================== Files 323 323 Lines 27125 27144 +19 Branches 2223 2227 +4 =============================================== + Hits 18891 18901 +10 - Misses 8109 8118 +9 Partials 125 125 ``` | [Files](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute) | Coverage Δ | | |---|---|---| | [...components/explore/DifferentialExpressionPanel.jsx](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070?src=pr&el=tree&filepath=app%2Fjavascript%2Fcomponents%2Fexplore%2FDifferentialExpressionPanel.jsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL2phdmFzY3JpcHQvY29tcG9uZW50cy9leHBsb3JlL0RpZmZlcmVudGlhbEV4cHJlc3Npb25QYW5lbC5qc3g=) | `68.65% <100.00%> (ø)` | | | [.../components/explore/ExploreDisplayPanelManager.jsx](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070?src=pr&el=tree&filepath=app%2Fjavascript%2Fcomponents%2Fexplore%2FExploreDisplayPanelManager.jsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL2phdmFzY3JpcHQvY29tcG9uZW50cy9leHBsb3JlL0V4cGxvcmVEaXNwbGF5UGFuZWxNYW5hZ2VyLmpzeA==) | `59.43% <100.00%> (ø)` | | | [...vascript/components/explore/ExploreDisplayTabs.jsx](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070?src=pr&el=tree&filepath=app%2Fjavascript%2Fcomponents%2Fexplore%2FExploreDisplayTabs.jsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL2phdmFzY3JpcHQvY29tcG9uZW50cy9leHBsb3JlL0V4cGxvcmVEaXNwbGF5VGFicy5qc3g=) | `60.13% <100.00%> (+0.28%)` | :arrow_up: | | [app/javascript/components/explore/ScatterTab.jsx](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070?src=pr&el=tree&filepath=app%2Fjavascript%2Fcomponents%2Fexplore%2FScatterTab.jsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL2phdmFzY3JpcHQvY29tcG9uZW50cy9leHBsb3JlL1NjYXR0ZXJUYWIuanN4) | `72.36% <100.00%> (+0.36%)` | :arrow_up: | | [.../javascript/components/visualization/PlotTitle.jsx](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070?src=pr&el=tree&filepath=app%2Fjavascript%2Fcomponents%2Fvisualization%2FPlotTitle.jsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL2phdmFzY3JpcHQvY29tcG9uZW50cy92aXN1YWxpemF0aW9uL1Bsb3RUaXRsZS5qc3g=) | `76.47% <100.00%> (ø)` | | | [...ion/controls/DifferentialExpressionGroupPicker.jsx](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070?src=pr&el=tree&filepath=app%2Fjavascript%2Fcomponents%2Fvisualization%2Fcontrols%2FDifferentialExpressionGroupPicker.jsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL2phdmFzY3JpcHQvY29tcG9uZW50cy92aXN1YWxpemF0aW9uL2NvbnRyb2xzL0RpZmZlcmVudGlhbEV4cHJlc3Npb25Hcm91cFBpY2tlci5qc3g=) | `53.09% <100.00%> (ø)` | | | [...nents/visualization/controls/ScatterPlotLegend.jsx](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070?src=pr&el=tree&filepath=app%2Fjavascript%2Fcomponents%2Fvisualization%2Fcontrols%2FScatterPlotLegend.jsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL2phdmFzY3JpcHQvY29tcG9uZW50cy92aXN1YWxpemF0aW9uL2NvbnRyb2xzL1NjYXR0ZXJQbG90TGVnZW5kLmpzeA==) | `58.64% <100.00%> (-0.62%)` | :arrow_down: | | [app/javascript/lib/plot.js](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070?src=pr&el=tree&filepath=app%2Fjavascript%2Flib%2Fplot.js&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL2phdmFzY3JpcHQvbGliL3Bsb3QuanM=) | `73.99% <100.00%> (ø)` | | | [test/js/visualization/scatter-plot.test-data.js](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070?src=pr&el=tree&filepath=test%2Fjs%2Fvisualization%2Fscatter-plot.test-data.js&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-dGVzdC9qcy92aXN1YWxpemF0aW9uL3NjYXR0ZXItcGxvdC50ZXN0LWRhdGEuanM=) | `100.00% <100.00%> (ø)` | | | [...avascript/components/visualization/ScatterPlot.jsx](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070?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.43% <60.78%> (+1.45%)` | :arrow_up: | ... and [9 files with indirect coverage changes](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2070/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 4 months ago

The build failure seems to be a true positive. I'll be available after demo if you'd like to pair!

Total elapsed time: 25 minutes, 54 seconds

### FAILURES and ERRORS ###
*** Yarn test failures ***
  FAIL test/js/explore/differential-expression-panel.test.js (14.966 s)
      ✕ renders DE genes table (379 ms)
      ✕ renders pairwise group picker (61 ms)
  FAIL test/js/explore/differential-expression-panel.test.js (14.966 s)

*** Rails test failures ***
Exiting with code: 1

Yeah, on it - pretty sure I know what I broke in the test.

bistline commented 4 months ago

Build failure now is unrelated, upstream Terra issue.

bistline commented 4 months ago

I just discovered a bug with this... everything works fine on initial load where you add spatial plots iteratively, but once they're all loaded it breaks on annotation switch or page reload. This is because the spatial plots might render before the main plot, and the refColorMap changes after they've rendered, leading to color mismatches across or even within a plot (e.g. spatial plot has different colors for trace and legend).

bistline commented 3 months ago

@eweitz @jlchang this is ready for review now, I think I've get the state issue fixed.

eweitz commented 3 months ago

I created SCP405 on staging to reproduce SCP1375 from production, for convenient reference.