Y24-190-2: Use ss v2 for labware metadata #1847
Conversation
Only just found out about it and it looks like it hasn't been done by anyone in recent times.
It's used by sub-classes so they need to be fixed first
…etadata # Conflicts: # spec/models/labware_creators/pcr_cycles_binned_plate_for_t_nano_seq_spec.rb
This includes creating a new module which lets LabwareCreators lookup a V2 plate as a `source_plate` when they usually only deal with v1 plates as `parent`.
They no longer use the api V1 and the tests were passing them the api variable when they no longer accept one.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-Y24-190 #1847 +/- ##
===================================================
+ Coverage 91.48% 91.55% +0.06%
===================================================
Files 380 382 +2
Lines 7738 7813 +75
===================================================
+ Hits 7079 7153 +74
- Misses 659 660 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @source_plate = nil | ||
| end | ||
|
|
||
| @source_plate |
There was a problem hiding this comment.
What does the instance variable at the bottom do? How does it work when you have both a function and instance variable with the same name?
There was a problem hiding this comment.
overriding the getter for the instance variable to fetch the plate using v2?
There was a problem hiding this comment.
It's a pattern called memoization where you use the @ variable to store a cached value to avoid having to perform the lookup repeatedly for the parent UUID. Rubocop requires that you use the same name for the @ variable as you do for the method that populates it. See a blog article on memoization here: https://blog.appsignal.com/2022/12/20/a-guide-to-memoization-in-ruby.html
When you call self.source_plate from elsewhere in the class (and publically in this case), the method will be preferred over the @ variable.
There was a problem hiding this comment.
Ah ok cool, thanks I didn't realise you could do it this way. I think I've used a similar way before something like:
def source_plate
@ source_plate ||= <method_call>
end
There was a problem hiding this comment.
I think that's what I'm doing here as well. It's just complicated by the fact that we have to check the type first before converting it and an if check to ensure we do over-write our memoized version if, somehow, the parent has changed.
|
|
||
| stub_v2_user(user) | ||
|
|
||
| stub_v2_labware(stock_plate_v2) |
There was a problem hiding this comment.
Is there a nice way of handling this where we can give it a list of plates instead of invoking it for each plate.
There was a problem hiding this comment.
We could add more stub methods, but there are already a lot of them. We could use some sort of nasty reflection method where we tell it to stub resources and pass the type as a string, constantizing it to get a reference to the individual stub method and looping inside the stubs module. Or we could set this up something like:
[child_plate_a, child_plate_b, child_plate_c, child_plate_d, child_plate_e].each { |labware| stub_v2_labware(labware) }
But as these are tests, do not need to conform to DRY approaches, and I want the setup to be very clear to anyone who needs to understand what the setup and outcomes are, I don't think we should do any of that.
| @@ -93,10 +96,10 @@ def metadata_stock_barcode | |||
| end | |||
|
|
|||
| def parent_metadata | |||
| if parent.is_a? Limber::Plate | |||
There was a problem hiding this comment.
There is the concept of a plate meant to be used in Limber and one only meant to be used in Sequencescape. You get a warning in the GUI if you scan a non-Limber plate in Limber.
Is this Limber::Plate check to do with that? And why do you replace it here but then use it in the support_v2_plate_source_plate code above?
There was a problem hiding this comment.
Maybe the method source_plate is badly named. I took that from code James Glover wrote for cardinal pools plate labware creator to convert a parent into a V2 plate. Not all the labware creators deal with plates I believe since one of the tests fails if I don't check that we have a plate here. In which case it falls back to passing the parent's barcode to the LabwareMetadata. I guess when it's a tube, as it is in one of the tests, it gives the barcode of the tube to LabwareMetadata and the labware gets looked up that way. I don't know if there are any real life instances where the labware would not be able to be looked up by its barcode. I assumed so because of the dual functionality constructor that can take the labware itself or the barcode. But perhaps that's legacy functionality and we could make our lives a lot easier by only using the barcodes all the time?
There was a problem hiding this comment.
I think I've change this now to be cleaner.
Either it's the correct type, or it's nil, which is when we would fall back to the barcode instead for LabwareMetadata
Closes #1803
Changes proposed in this pull request
Instructions for Reviewers
[All PRs] - Confirm PR template filled
[Feature Branches] - Review code