feat(api, shared-data): add dynamic pipetting into liquid classes

[ryanthecoder]

Overview

This PR encompases a first pass implementation of allowing dynamic tracking with liquid classes. As is there is no super convenient way a new way for users to edit positions in transfer properties. There is no default selection for any current liquid classes and is defaulted to None everywhere.

But as a first pass this does add a few new features

• TipPosition has split LIQUID_MENISCUS into LIQUID_MENISCUS_START and LIQUID_MENISCUS_END to match the implementation with the standard aspirate and dispense options
• Adds a Schema 2 liquid class which adds aspirate_end_position and dispense_end_position as optional members of the liquid class, it also adds override start and end positions that can be used, and will fallback to the old positions if they throw an error caused by no liquid level detection data, or inner well geometry data.
• Enables the use of dynamic tracking in liquid classes for the gravimetric protocol which will be part of another PR and the minimal implementation of that is enabled for testing
• Adds a convenience method to the aspirate, single-dispense and multi-dispense objects within transfer properties to add an override tip position

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.35%. Comparing base (9eef3bd) to head (8b58a4c).
⚠️ Report is 45 commits behind head on edge.

Additional details and impacted files

@@             Coverage Diff             @@
##             edge   #20232       +/-   ##
===========================================
- Coverage   56.46%   25.35%   -31.12%     
===========================================
  Files        3626     3632        +6     
  Lines      301067   302987     +1920     
  Branches    42309    42633      +324     
===========================================
- Hits       170002    76825    -93177     
- Misses     130839   226146    +95307     
+ Partials      226       16      -210     

Flag	Coverage Δ
protocol-designer	19.29% <ø> (+0.04%)	⬆️
step-generation	5.88% <ø> (+0.28%)	⬆️

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

Files with missing lines	Coverage Δ
...n/opentrons_shared_data/liquid_classes/__init__.py	81.81% <ø> (-2.80%)	⬇️
...red_data/liquid_classes/liquid_class_definition.py	95.83% <ø> (-0.07%)	⬇️
...thon/opentrons_shared_data/liquid_classes/types.py	0.00% <ø> (ø)

... and 1820 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sfoster1]

Some inline things; I think we should get naming consistent between defs and code, I think there's some stuff accidentally left in the pr.

In general I'm a little uneasy about the specificity of this implementation. There's a tip position and an override tip position, and the only way to go from override to backup is if specific exceptions about not having liquid height estimation data are thrown. That definitely works for the specific issue where maybe we don't have height data for a variety of reasons. But there's also more reasons we might want different tip positions:

• Maybe we trust filtertips, or some future filter tips, less
• Maybe we want to do different things depending on the volume in the well (for instance, maybe we find some wells where under a certain volume we know our estimations are unreliable)
  Adding different kinds of reasons to pick a different position is something that would either take a lot of refactoring from here or require more and more little checks and exceptions to be raised from lower layers and more and more specific named entries in the classes as fallbacks.

That said, I don't know if we need to go all the way to having an embedded conditional in the defs like previously proposed. I think we should give the new position groups some structure. Rather than having the flat entries aspiratePosition, aspirateEndPosition, overrideAspiratePosition, overrideAspirateEndPosition side by side, have aspiratePositions: [{name, start, end}, {name, start, end}] where name is some name like "dynamic" or "static". Then, the implied logic (and the specification in the class schema) is that the code will try and use these positions in order from the array, and if there's an exception go to the next.

That at least means that if we want to add more entries we won't need more schema changes; it avoids the thing I hate most in schemas which is flat entries with specific names, which is really just putting structure in element names when it could be in actual structure; and it gives some nicer specifiable names to things.

[sfoster1]

this doesn't seem right? there's no corresponding change in the engine aspirate command or dispense command, did there used to be?

[sfoster1]

we shouldn't need to change the v1 fixtures, right?

[sfoster1]

we shouldn't need to change the v1 schema, right?

[sfoster1]

it's a bit confusing that it's endPosition and overrideEndPosition in the class defs, but end_position and fallback_end_position in the code. can we make these the same?

[sfoster1]

debug print to remove

[sfoster1]

this is weird, it duplicates the code that's in aspirateWhileTracking doesn't it? i thought we had the capability to call aspirateWhileTracking with end position meniscus-end AND a user-provided offset?

[sfoster1]

or is the intent to force an exception to raise here rather than in the command? if so, can we add an engine query function that lets us say "hey can you do meniscus-end for this well right now"

[sfoster1]

just for diff improvements and indentation levels, instead of the recursion can we

• move the match statement into its own function _match_absolute_point... or something
• change absolute_point_from_position... to first call _match_absolute_point... with the primary, catch the exception, and then call it with the fallback?

[ddcc4]

Note to self/@jbleon95: I haven't read through all of changes yet, but I don't see any edits the public transfer functions in instrument_context.py. I think (?) Ryan is proposing to add both static positions and meniscus-relative positions to the liquid class definitions. If both are present, how does the user choose which to use?