-
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
feat(engine): consolidate_liquid
engine core implementation
#17458
base: edge
Are you sure you want to change the base?
Changes from all commits
f4c25c2
7e011b0
9ef3682
2862106
38f8f20
d97ebae
6a802fe
6012d02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,7 +144,7 @@ def submerge( | |
minimum_z_height=None, | ||
speed=None, | ||
) | ||
if self._transfer_type == TransferType.ONE_TO_ONE: | ||
if self._transfer_type != TransferType.ONE_TO_MANY: | ||
self._remove_air_gap(location=submerge_start_location) | ||
self._instrument.move_to( | ||
location=self._target_location, | ||
|
@@ -217,7 +217,10 @@ def mix(self, mix_properties: MixProperties, last_dispense_push_out: bool) -> No | |
NOTE: For most of our built-in definitions, we will keep _mix_ off because it is a very application specific thing. | ||
We should mention in our docs that users should adjust this property according to their application. | ||
""" | ||
if not mix_properties.enabled: | ||
if ( | ||
not mix_properties.enabled | ||
or self._transfer_type == TransferType.MANY_TO_ONE | ||
): | ||
Comment on lines
+220
to
+223
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, this is my fault; the docstring I wrote is misleading. I think we do want to run the post-dispense mix, while skipping the pre-aspirate mix. One way to do that is do the gating inside |
||
return | ||
# Assertion only for mypy purposes | ||
assert ( | ||
|
@@ -248,7 +251,10 @@ def pre_wet( | |
- No push out | ||
- No pre-wet for consolidation | ||
""" | ||
if not self._transfer_properties.aspirate.pre_wet: | ||
if ( | ||
not self._transfer_properties.aspirate.pre_wet | ||
or self._transfer_type == TransferType.MANY_TO_ONE | ||
): | ||
return | ||
mix_props = MixProperties(_enabled=True, _repetitions=1, _volume=volume) | ||
self.mix(mix_properties=mix_props, last_dispense_push_out=False) | ||
|
@@ -313,9 +319,15 @@ def retract_after_aspiration(self, volume: float) -> None: | |
# Full speed because the tip will already be out of the liquid | ||
speed=None, | ||
) | ||
# For consolidate, we need to know the total amount that is in the pipette since this | ||
# may not be the first aspirate | ||
if self._transfer_type == TransferType.MANY_TO_ONE: | ||
volume_for_air_gap = self._instrument.get_current_volume() | ||
else: | ||
volume_for_air_gap = volume | ||
self._add_air_gap( | ||
air_gap_volume=self._transfer_properties.aspirate.retract.air_gap_by_volume.get_for_volume( | ||
volume | ||
volume_for_air_gap | ||
) | ||
) | ||
|
||
|
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.
We should highlight this and mix, pre-wet behavior in docs. I guess it's okay to raise this error instead of simply skipping the blowout, as long as our built-in classes don't have blowout with source location enabled by default, because otherwise a lot of protocols will run into errors due to no fault of the users. Liquid classes are just supposed to work without having to modify for each transfer.