DPL-914 scRNA create tube rack (plate) from banked pmbc tubes #1586

andrewsparkes · 2024-02-16T16:45:13Z

Changes proposed in this pull request

Adds the labware creator for making the scRNA cryostor plate from the Seq and Spare Bank tubes using a tube rack scan

Instructions for Reviewers

[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to main]
- Check story numbers included
- Check for debug code
- Check version

codecov · 2024-02-26T16:21:21Z

Codecov Report

Attention: Patch coverage is 96.51163% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 90.78%. Comparing base (4879b00) to head (47cad6a).

Files	Patch %	Lines
app/models/state_changers.rb	72.72%	3 Missing ⚠️
...creators/common_file_handling/csv_file/row_base.rb	85.71%	2 Missing ⚠️
.../models/labware_creators/pooled_tubes_by_sample.rb	0.00%	2 Missing ⚠️
app/sequencescape/sequencescape/api/v2.rb	33.33%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1586      +/-   ##
===========================================
- Coverage    90.82%   90.78%   -0.05%     
===========================================
  Files          359      362       +3     
  Lines         7424     7452      +28     
===========================================
+ Hits          6743     6765      +22     
- Misses         681      687       +6

Flag	Coverage Δ
javascript	`93.65% <ø> (ø)`
pull_request	`90.78% <96.51%> (?)`
push	`90.78% <96.51%> (-0.05%)`	⬇️
ruby	`90.37% <96.51%> (-0.06%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

app/models/labware_creators/multi_stamp_tubes_using_tube_rack_scan/csv_file/row.rb

app/models/labware_creators/multi_stamp_tubes_using_tube_rack_scan.rb

app/models/labware_creators/multi_stamp_tubes_using_tube_rack_scan/csv_file.rb

…hild_plate_v1

KatyTaylor

Haven't looked at the state changer or api stuff yet, but submitting these comments in a batch as I might not get a chance to finish reviewing until later this afternoon.

KatyTaylor · 2024-03-04T11:24:57Z

app/models/labware_creators/multi_stamp_tubes_using_tube_rack_scan.rb

+  # type(s).
+  # 5) The request on the tube must be active, and must match to one of the expected ones from a list in the plate
+  # purpose config. This is to check that the tubes are at the appropriate stage of their pipeline to transfer.
+  #


Nice docs, thank you.

KatyTaylor · 2024-03-04T11:37:56Z

app/models/labware_creators/multi_stamp_tubes_using_tube_rack_scan.rb

Really nice and easy to read, thanks for the sensible method order and informative comments.

KatyTaylor · 2024-03-04T11:40:14Z

app/models/labware_creators/multi_stamp_tubes_using_tube_rack_scan/csv_file.rb

Definitely some code duplication going on here - did you consider whether to try to share code with other CsvFile classes?

yes can have a look, was having the same in the Targeted NanoSeq stories. Maybe can create some common classes for all of these file upload classes.

KatyTaylor · 2024-03-04T11:43:02Z

app/models/labware_creators/multi_stamp_tubes_using_tube_rack_scan/csv_file/row.rb

Same here - code duplication. Might be easier to refactor considering the thinking you put into it already in the last similar story?

app/views/plate_creation/multi_stamp_tubes_using_tube_rack_scan.html.erb

KatyTaylor · 2024-03-04T11:46:55Z

app/views/plate_creation/multi_stamp_tubes_using_tube_rack_scan.html.erb

Would be good to have a standard 'upload tube rack scan file' view (maybe a partial?) that we could reuse, as we now have these uploads in several different places. Could break that out into a different technical debt story though maybe, as it might take some thinking.

The other one in this pipeline has 2 files, for contingency and sequencing racks.
Agree it should be possible to make a common one, maybe that allows for an array of files. Set up from attributes in the labware creator.

KatyTaylor

Left some comments. The API stuff looks like a good change to me. I agree we should be cautious though and make sure the Integration Suite tests pass.

app/sequencescape/sequencescape/api/v2/shared/has_requests.rb

app/sequencescape/sequencescape/api/v2/tube.rb

app/sequencescape/sequencescape/api/v2.rb

…n.html.erb Co-authored-by: KatyTaylor <[email protected]>

…thub.com:sanger/limber into dpl-914-scrna-create-tube-rack-banked-pmbc-tubes

app/models/labware_creators/common_file_handling/csv_file_for_tube_rack.rb

KatyTaylor

Thanks for doing all that refactoring. I've left a few extra comments but they're all minor.

app/models/labware_creators/common_file_handling/csv_file/row_base.rb

KatyTaylor · 2024-03-11T11:43:46Z

app/models/labware_creators/common_file_handling/csv_file/row_for_tube_rack.rb

+    #
+    class CsvFile::RowForTubeRack < CsvFile::RowBase
+      TUBE_LOCATION_NOT_RECOGNISED = 'contains an invalid coordinate, in %s'
+      TUBE_BARCODE_MISSING = 'cannot be empty, in %s'


The error messages could flow better, I think, but I guess you are copy pasting from an existing class? What does it end up as, something like:

Tube barcode cannot be empty, in CsvFile::RowForTubeRack

?

The 'in' part is the to_s method that gives you a row number reference (plus well coord if available)
e.g. Tube barcode cannot be empty, in row 11 [G5]

KatyTaylor · 2024-03-11T13:02:23Z

app/models/labware_creators/common_file_handling/csv_file_base.rb

+
+    # Override in subclass if needed
+    def reset_variables
+      @parent_barcode = nil


Not sure why @parent_barcode is being reset, do you know? The other 3 are initialized in this class.

well spotted, removed
cut and paste error from the pcr cycles version of csv file base

app/models/labware_creators/common_file_handling/csv_file_for_tube_rack.rb

KatyTaylor · 2024-03-11T13:11:04Z

app/models/labware_creators/common_file_handling/csv_file_for_tube_rack.rb

+    # Generates a hash of position details based on the tube rack scan data in the CSV file.
+    #
+    # @return [Hash] A hash of position details, where the keys are positions and the values
+    # are hashes containing the tube rack barcode and tube barcode for each position.


Suggested change

# are hashes containing the tube rack barcode and tube barcode for each position.

# are hashes containing the tube barcode for each position.

Is this correct? Does not contain the tube rack barcode?

correct, fixed

KatyTaylor · 2024-03-11T13:15:51Z

app/models/labware_creators/pcr_cycles_binned_plate/csv_file/row_base.rb

-      @row_data = row_data
+      super(index, row_data)
+
+      # initialize pipeline specific columns


Suggested change

# initialize pipeline specific columns

yoldas

Added some comments. I will continue in a new batch.

app/models/labware_creators/common_file_handling/csv_file_base.rb

app/models/labware_creators/common_file_handling/csv_file/row_base.rb

app/models/labware_creators/common_file_handling/csv_file_for_tube_rack.rb

app/models/labware_creators/custom_pooled_tubes/csv_file/row.rb

yoldas

One more comment.

spec/models/labware_creators/common_file_handling/csv_file_for_tube_rack_spec.rb

…base.rb Co-authored-by: KatyTaylor <[email protected]>

…c-tubes

codeclimate · 2024-03-21T15:00:26Z

Code Climate has analyzed commit 47cad6a and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	3

The test coverage on the diff in this pull request is 96.5% (50% is the threshold).

This pull request will bring the total coverage in the repository to 90.3% (-0.1% change).

View more on Code Climate.

andrewsparkes added 19 commits February 16, 2024 15:11

added automatic state changer for tubes

ddca014

added csv file and row for tube rack scan

fec6cdd

added purpose config factory

3eee14f

added custom page to upload tube rack scan

4f86957

added config changes for purposes

f2cac66

refactor to make easier to read

22084d2

refactor to make easier to read 2

9f62936

added delegates

11579a0

added helper methods for tubes

64d3dff

added test csv files

94af82f

added relationship to request_type on v2 request

da72890

added requests_as_source to factory

42aaed0

refactor to match plate version

95c9cda

initial version of labware creator and partial test - wip

bfbabb5

doc updates

5657947

added cached relationships into factories

1979d99

spec test files for multi stamp tubes tests

4677de4

added tests for the various validations

bf50c49

updated docs

3f17011

andrewsparkes linked an issue Feb 26, 2024 that may be closed by this pull request

DPL-914 Create tube rack containing banked PBMC tubes #1388

Closed

3 tasks

merged in develop

e822cae

fix tests

bad5a3b

andrewsparkes marked this pull request as ready for review February 26, 2024 17:09

andrewsparkes added 6 commits February 27, 2024 11:25

removed unneeded relationship

4aa152b

modified includes and has requests to fetch in progress requests

215bc5d

update docs

9f9d755

modified test to use ancestor requests

28cf1cb

fix codepilot change

2bf79b0

added class type plate

583370c

moved v2 aliquots into v2 receptacle

fa569e7

codeclimate bot reviewed Mar 1, 2024

View reviewed changes

andrewsparkes and others added 4 commits March 1, 2024 12:56

WIP: file upload not working

82d4a80

added missing requires and uploader

e69c18b

Fix file uploading in test by adding API V1 actions for qc_files to c…

d357dd4

…hild_plate_v1

added test for runtime error check

174f424

andrewsparkes self-assigned this Mar 4, 2024

KatyTaylor reviewed Mar 4, 2024

View reviewed changes

KatyTaylor reviewed Mar 5, 2024

View reviewed changes

app/sequencescape/sequencescape/api/v2/shared/has_requests.rb Outdated Show resolved Hide resolved

app/sequencescape/sequencescape/api/v2/tube.rb Show resolved Hide resolved

app/sequencescape/sequencescape/api/v2.rb Outdated Show resolved Hide resolved

KatyTaylor mentioned this pull request Mar 5, 2024

DPL-916 PBMC Donor Pooling create plate #1582

Merged

andrewsparkes and others added 7 commits March 5, 2024 16:42

Update app/views/plate_creation/multi_stamp_tubes_using_tube_rack_sca…

3c12d57

…n.html.erb Co-authored-by: KatyTaylor <[email protected]>

refactored to reduce code duplication for tube rack scan files

a40de67

Merge branch 'dpl-914-scrna-create-tube-rack-banked-pmbc-tubes' of gi…

01456df

…thub.com:sanger/limber into dpl-914-scrna-create-tube-rack-banked-pmbc-tubes

removed unneeded method

d592564

fixed comment

9a2c6d7

refactor of csv file upload handling

3d20dd7

fix initialisations and file paths

a3f9b75

codeclimate bot reviewed Mar 8, 2024

View reviewed changes

app/models/labware_creators/common_file_handling/csv_file_for_tube_rack.rb Show resolved Hide resolved

KatyTaylor approved these changes Mar 11, 2024

View reviewed changes

yoldas reviewed Mar 11, 2024

View reviewed changes

spec/models/labware_creators/common_file_handling/csv_file_for_tube_rack_spec.rb Show resolved Hide resolved

andrewsparkes and others added 3 commits March 11, 2024 15:24

Update app/models/labware_creators/common_file_handling/csv_file/row_…

19a7545

…base.rb Co-authored-by: KatyTaylor <[email protected]>

changes after feedback

d383ea8

merged develop in

1f3329c

yoldas approved these changes Mar 11, 2024

View reviewed changes

yoldas and others added 2 commits March 18, 2024 17:02

Change tube.aliquots to tube.receptacle.aliquots in includes

2114559

Merge branch 'develop' into dpl-914-scrna-create-tube-rack-banked-pmb…

47cad6a

…c-tubes

andrewsparkes merged commit 3a28ac1 into develop Mar 22, 2024
12 checks passed

andrewsparkes deleted the dpl-914-scrna-create-tube-rack-banked-pmbc-tubes branch March 22, 2024 11:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DPL-914 scRNA create tube rack (plate) from banked pmbc tubes #1586

DPL-914 scRNA create tube rack (plate) from banked pmbc tubes #1586

andrewsparkes commented Feb 16, 2024 •

edited

Loading

codecov bot commented Feb 26, 2024 •

edited

Loading

KatyTaylor left a comment

KatyTaylor Mar 4, 2024

KatyTaylor Mar 4, 2024

KatyTaylor Mar 4, 2024

andrewsparkes Mar 5, 2024

KatyTaylor Mar 4, 2024

andrewsparkes Mar 5, 2024

KatyTaylor Mar 4, 2024

andrewsparkes Mar 5, 2024

KatyTaylor left a comment

KatyTaylor left a comment

KatyTaylor Mar 11, 2024

andrewsparkes Mar 11, 2024

KatyTaylor Mar 11, 2024

andrewsparkes Mar 11, 2024

KatyTaylor Mar 11, 2024

andrewsparkes Mar 11, 2024

KatyTaylor Mar 11, 2024

yoldas left a comment

yoldas left a comment

codeclimate bot commented Mar 21, 2024

	# are hashes containing the tube rack barcode and tube barcode for each position.
	# are hashes containing the tube barcode for each position.

DPL-914 scRNA create tube rack (plate) from banked pmbc tubes #1586

DPL-914 scRNA create tube rack (plate) from banked pmbc tubes #1586

Conversation

andrewsparkes commented Feb 16, 2024 • edited Loading

Changes proposed in this pull request

Instructions for Reviewers

codecov bot commented Feb 26, 2024 • edited Loading

Codecov Report

KatyTaylor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KatyTaylor left a comment

Choose a reason for hiding this comment

KatyTaylor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoldas left a comment

Choose a reason for hiding this comment

yoldas left a comment

Choose a reason for hiding this comment

codeclimate bot commented Mar 21, 2024

andrewsparkes commented Feb 16, 2024 •

edited

Loading

codecov bot commented Feb 26, 2024 •

edited

Loading