Skip to content

Make updating existing DE result more robust (SCP-5952)#2228

Merged
bistline merged 6 commits intodevelopmentfrom
jb-de-cluster-name-ref
Mar 27, 2025
Merged

Make updating existing DE result more robust (SCP-5952)#2228
bistline merged 6 commits intodevelopmentfrom
jb-de-cluster-name-ref

Conversation

@bistline
Copy link
Copy Markdown
Contributor

BACKGROUND & CHANGES

This update makes the process of updating existing DifferentialExpressionResult objects more robust. Previously, there were many corners where finding the associate matrix file or clustering object didn't work properly, which would cause results creation to fail validation because associations weren't set, or it attempted to create a new result object when one already existed. Now, rather than finding associations by matching names, cluster_group_id and matrix_file_id enable direct association mapping without ambiguity. Convenience methods of cluster_group and matrix_file also allow loading the associated objects easier. There are also new convenience methods for finding existing DE results. Lastly, in the case of an existing result where the associated clustering object has been renamed, the cached name (i.e. result.cluster_name) is passed to ingest so that all output files for an existing result are consistent. These updates together simplify the logic in IngestJob#create_differential_expression_results.

MANUAL TESTING

  1. Boot all services and sign in
  2. Go to any study that has existing one-vs-rest DE results
  3. Open the upload wizard for this study and rename the cluster that has existing results (e.g. Epithelial Cells UMAP to. Epithelial Cells)
  4. Open the DE submission swagger form and request new pairwise calculations for the existing result
  5. In development.log, find the corresponding Batch API call and note that the original name for the cluster was passed for cluster_name (this makes output filenames match the existing results)
  6. Once the job succeeds, confirm you can load the new pairwise results (you will need to reload the page as the /explore response is cached)

@bistline bistline requested review from eweitz and jlchang March 26, 2025 14:26
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2025

Codecov Report

Attention: Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.

Project coverage is 70.63%. Comparing base (b049c7c) to head (920fedc).
Report is 11 commits behind head on development.

Files with missing lines Patch % Lines
app/lib/differential_expression_service.rb 88.88% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #2228   +/-   ##
============================================
  Coverage        70.62%   70.63%           
============================================
  Files              331      331           
  Lines            28292    28301    +9     
  Branches          2464     2464           
============================================
+ Hits             19981    19990    +9     
  Misses            8164     8164           
  Partials           147      147           
Files with missing lines Coverage Δ
app/models/differential_expression_parameters.rb 100.00% <100.00%> (ø)
app/models/differential_expression_result.rb 92.24% <100.00%> (ø)
app/models/ingest_job.rb 55.76% <100.00%> (-0.14%) ⬇️
app/lib/differential_expression_service.rb 91.70% <88.88%> (-0.23%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@eweitz eweitz left a comment

Choose a reason for hiding this comment

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

Code looks good!

Copy link
Copy Markdown
Contributor

@jlchang jlchang left a comment

Choose a reason for hiding this comment

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

Functional review - works as described.

Glad to see DifferentialExpressionResult.find_existing_result, should be quite useful!

@bistline bistline merged commit 07a327c into development Mar 27, 2025
4 of 5 checks passed
@github-actions github-actions bot deleted the jb-de-cluster-name-ref branch March 27, 2025 14:25
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.

3 participants