refactor(api, shared-data): rename certain liquid class properties#17310
refactor(api, shared-data): rename certain liquid class properties#17310
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## edge #17310 +/- ##
===========================================
+ Coverage 25.45% 61.53% +36.07%
===========================================
Files 3018 3030 +12
Lines 234749 237525 +2776
Branches 20157 21178 +1021
===========================================
+ Hits 59767 146156 +86389
+ Misses 174966 91190 -83776
- Partials 16 179 +163
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
emilyburghardt
left a comment
There was a problem hiding this comment.
Just adding a comment- will leave actual approval to Sanniti, etc. on the actual changes.
I think the changes to position reference - start, end, and aspirate/dispense position reference- are going to make a huge difference in user understanding. I had it worked out while just documenting this, but especially when it comes to users editing their own liquid class... will be super helpful.
sanni-t
left a comment
There was a problem hiding this comment.
This looks great! The new positioning structure gives much more clarity.
Just have some suggestions for the position descriptions.
| const { | ||
| positionReference: aspiratePositionReference, | ||
| offset: aspirateOffset, | ||
| } = aspirate.aspiratePosition |
There was a problem hiding this comment.
nit, can you deconstruct aspiratePosition from aspirate like in line 682 and then this could be:
| const { | |
| positionReference: aspiratePositionReference, | |
| offset: aspirateOffset, | |
| } = aspirate.aspiratePosition | |
| const { | |
| positionReference: aspiratePositionReference, | |
| offset: aspirateOffset, | |
| } = aspiratePosition |
its just cleaner this way to read and we prefer this :D
| const { | ||
| positionReference: dispensePositionReference, | ||
| offset: dispenseOffset, | ||
| } = dispense.dispensePosition |
| flowRateByVolume: aspirateFlowRateByVolume, | ||
| delay: aspirateDelay, | ||
| } = aspirate | ||
| const { positionReference, offset } = aspirate.aspiratePosition |
…17310) This PR refactors some liquid class property names and organization of properties, fully changing mmFromEdge to mmToEdge and re-organizing positionReference and offset properties.
…17310) This PR refactors some liquid class property names and organization of properties, fully changing mmFromEdge to mmToEdge and re-organizing positionReference and offset properties.
…17310) This PR refactors some liquid class property names and organization of properties, fully changing mmFromEdge to mmToEdge and re-organizing positionReference and offset properties.
…17310) This PR refactors some liquid class property names and organization of properties, fully changing mmFromEdge to mmToEdge and re-organizing positionReference and offset properties.
…17310) This PR refactors some liquid class property names and organization of properties, fully changing mmFromEdge to mmToEdge and re-organizing positionReference and offset properties.
…17310) This PR refactors some liquid class property names and organization of properties, fully changing mmFromEdge to mmToEdge and re-organizing positionReference and offset properties. (cherry picked from commit 42fbc0d624d5c134d9f8826bcb2e745f8c1c2934)
…17310) This PR refactors some liquid class property names and organization of properties, fully changing mmFromEdge to mmToEdge and re-organizing positionReference and offset properties. (cherry picked from commit d82f3211888af075cd2b635aad5f7f125f937f17)
…17310) This PR refactors some liquid class property names and organization of properties, fully changing mmFromEdge to mmToEdge and re-organizing positionReference and offset properties. (cherry picked from commit d82f3211888af075cd2b635aad5f7f125f937f17)
…17310) This PR refactors some liquid class property names and organization of properties, fully changing mmFromEdge to mmToEdge and re-organizing positionReference and offset properties.
Overview
This PR refactors some liquid class property names and organization of properties. The two major changes are fully changing
mmFromEdgetommToEdgeand re-organizingpositionReferenceandoffsetproperties.mmFromEdgetommToEdgewas already partially in the code in some places, so this change just fully changes it everywhere else. This name is viewed as more readable and understandable, as the property is measured from the edge's well.The larger change was to
positionReferenceandoffset. Previously these were two properties that were flatly defined in thesubmergefield, theretractfield and in the top levelaspirate,singleDispenseandmultiDispensefields. This refactor changes it so that these two properties are now bound together in a newTipPositiontype, and named appropriately and descriptively in each of the above fields.For
submergethese are now undersubmerge.start_position. Forretract, it is now underretract.end_position. Foraspirateit is now underaspirate.aspirate_positionand forsingleDispenseandmultiDispenseit is now underfooDispense.dispense_position. All of these clearly highlight if the position reference and offset refers to the starting position, liquid handling position, or end position.Test Plan and Hands on Testing
Unit tests and updated snapshot tests should cover the changes, and ensured the following protocol passed analysis
Changelog
mmToEdgeandmm_to_edgetommFromEdgeandmm_from_edgepositionReferenceandoffsetto be in newtipPositiontype and named appropriately in transfer property fieldsReview requests
Risk assessment
Lowish, doesn't change any execution code but touches a lot of areas.