-
Notifications
You must be signed in to change notification settings - Fork 179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(api): Aspirate, dispense, and mix can perform at 0ul #13989
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## edge #13989 +/- ##
=======================================
Coverage 70.67% 70.67%
=======================================
Files 2505 2505
Lines 70832 70832
Branches 8738 8738
=======================================
Hits 50058 50058
Misses 18639 18639
Partials 2135 2135
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right or wrong, this is established behavior in currently-running protocol api versions and we cannot change it retroactively. Please gate this change behind being on api version 2.16 or above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy with functionality and docs if it passes tests but @ecormany would you mind taking a look at the docstrings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for writing up the docstrings to go with the new behavior!
A previous PR (12a630b / #13989 ) changed the python protocol api in version 2.16 to allow commanding 0ul liquid handling commands like aspirate, mix, and dispense. This is useful in programmatic protocols that read out volumes to handle; they can now handle 0 volume properly. In 2.15 and previous, specifying 0 would lead to those commands doing the most volume they could (i.e. aspirate the full tip volume, dispense whatever's currently in the pipette, mix at full volume) and this likely was an unintentional side effect because of python truthiness. However, that change only touched the python protocol API; that API would emit commands to the engine that specified 0 volume, and those were not allowed: the pydantic models for the commands and responses required strictly greater than 0 volume. This PR - changes the pydantic models and updates the schema to allow 0ul commands - adds a python protocol to be an integration test - adds unit tests for the python protocol api aspirate and dispense commands --------- Co-authored-by: Seth Foster <[email protected]> Co-authored-by: Max Marrone <[email protected]>
A previous PR (12a630b / #13989 ) changed the python protocol api in version 2.16 to allow commanding 0ul liquid handling commands like aspirate, mix, and dispense. This is useful in programmatic protocols that read out volumes to handle; they can now handle 0 volume properly. In 2.15 and previous, specifying 0 would lead to those commands doing the most volume they could (i.e. aspirate the full tip volume, dispense whatever's currently in the pipette, mix at full volume) and this likely was an unintentional side effect because of python truthiness. However, that change only touched the python protocol API; that API would emit commands to the engine that specified 0 volume, and those were not allowed: the pydantic models for the commands and responses required strictly greater than 0 volume. This PR - changes the pydantic models and updates the schema to allow 0ul commands - adds a python protocol to be an integration test - adds unit tests for the python protocol api aspirate and dispense commands --------- Co-authored-by: Seth Foster <[email protected]> Co-authored-by: Max Marrone <[email protected]>
Overview
I would expect
.aspirate(0)
and.dispense(0)
to aspirate/dispense 0 ul.Currently
.aspirate(0)
will aspirate the tip's max-volume, and.dispense(0)
will dispense all volume currently in the tip.Looking in git history and this line was added in instrument_context.py in 2020, where the
if not volume
is used to determine if the volume should be defaulted:I'm assuming this behavior wasn't intentional.
Test Plan
Changelog
Review requests
Risk assessment