Y24-313 scRNA pooling calculation changes #1951
Conversation
…uest metadata and use it in pooling calculations
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1951 +/- ##
===========================================
- Coverage 77.95% 77.92% -0.03%
===========================================
Files 459 460 +1
Lines 17692 17715 +23
Branches 225 225
===========================================
+ Hits 13791 13805 +14
- Misses 3899 3908 +9
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| source_wells.first&.aliquots&.first&.request&.request_metadata&.number_of_samples_per_pool || nil | ||
|
|
||
| if number_of_samples_per_pool.nil? | ||
| raise StandardError, 'Error: request_metadata.number_of_samples_per_pool is nil' |
There was a problem hiding this comment.
Note that if the option is not provided in submission or added to the request metadata using the rake task, this will cause a 500 error while creating the PBMC Pools plate. So, the number_of_samples_per_pool option is not optional.
There was a problem hiding this comment.
Which is correct, but maybe the message should be customer friendly like saying it should have been set on submission? Not sure how they'd fix that other than support issue.
| @@ -218,7 +238,7 @@ def tag_depth_hash | |||
| def build_pools | |||
| groups = split_single_group_by_study_and_project(source_wells_for_pooling) | |||
| groups = split_groups_by_unique_donor_ids(groups) | |||
| distribute_groups_across_pools(groups, number_of_pools) | |||
| distribute_groups_across_pools(groups, calculated_number_of_pools) | |||
There was a problem hiding this comment.
Note that distribute_groups_across_pools uses a halving algorithm in distribution at the moment, which causes uneven distribution in small number of pools. I think It needs a weighted sharing of pools to each group based on size first and then either a round robin or a fair sharing of remainder after integer division for distribution in them.
| # frozen_string_literal: true | ||
|
|
||
| # Represents a request metadatum in Limber via the Sequencescape API | ||
| class Sequencescape::Api::V2::RequestMetadata < Sequencescape::Api::V2::Base |
There was a problem hiding this comment.
metadatum in the comment is confusing I think. RequestMetadata is set to be singular for this resource (inflections, uncountable). I would avoid using both forms in the same context.
There was a problem hiding this comment.
I think I changed the class name and forgot the comment. Will fix.
|
Code Climate has analyzed commit 17dce0e and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 94.7% (50% is the threshold). This pull request will bring the total coverage in the repository to 91.1% (0.0% change). View more on Code Climate. |
Closes #1909