refactor(api): add pipette misc functions to protocol_api

[sanni-t]

Closes #2242

overview

This PR adds the following pipette functions to protocol_api:
touch_tip()
mix()
blow_out()
return_tip()
air_gap()
distribute()
consolidate()
transfer()

Adds a TransferPlan Class which will be used by distribute(), consolidate() and transfer() above.
The transfer actions are and the sequence of these actions is subject to change. New features of the transfer plan include ability to specify options/args of different pipette actions (eg. you can now specify location, presses and increment args of 'pick_up_tip()).
Features/ Bugs that will be addressed in the next PR:

1. displaying more readable commands strings- e.g, 'Wells A1, B1, C1 of ANSI 96 Standard Microplate on Slot 3' instead of current format- 'A1 of ANSI 96 Standard Microplate on 3, B1 of ANSI 96 Standard Microplate on 3, C1 of ANSI 96 Standard Microplate on 3..'
2. multichannel transfers - generating proper display strings for locations. e.g 'Column 1, Column 2' instead of 'A1, A2' (multichannel locations list gets edited to remove non first-row Wells and hence only first row wells are displayed as source/ dest location currently)
3. multichannel transfers - having the robot infer the column to move to when a non first-row well is given as the location
4. transfer between unequal number of sources and destinations
5. taking measures to always keep transfer volume > min_volume of pipette

review requests

I have tried to keep the methods true to their original behavior in the old api but have modified the args in cases where a Placeable was used before. This restricts the possible location arguments, e.g, in case of touch_tip(loc) which will now only accept a Well as a valid location. Please review these changes for any issues.

Test on a robot: run protocols in the app with the new protocol api flag set. Here are two protocols for reference
papi_instr_ctx_protocols.zip

And an example test if you want to test individual functions in shell:

>>> import asyncio
>>> import opentrons.types as t
>>> import opentrons.protocol_api as papi
>>> from opentrons.protocol_api import transfers as tx
>>> from opentrons import hardware_control as hc
>>> l = asyncio.get_event_loop()
>>> ctx = papi.ProtocolContext()
>>> api = l.run_until_complete(hc.API.build_hardware_controller(force=True))
>>> ad = hc.adapters.SynchronousAdapter(api)
>>> ctx.connect(ad)
>>> ctx.home()
>>> tiprack = ctx.load_labware_by_name('opentrons_96_tiprack_300_uL',1)
>>> instr = ctx.load_instrument('p300_single',t.Mount.LEFT,tip_racks=[tiprack])
>>> lw1 = ctx.load_labware_by_name('biorad_96_wellPlate_pcr_200_uL', 2)
>>> lw2 = ctx.load_labware_by_name('generic_96_wellPlate_380_uL', 3)
>>> xfer_plan = tx.TransferPlan(100, lw1.columns()[0], lw2.columns()[0],instr)
>>> for step in xfer_plan:
...     if isinstance(step['params'], dict):
...             getattr(instr, step['method'])(**step['params'])
...     else:
...             getattr(instr, step['method'])(*step['params'])

Add transfer options (you can borrow some options from tests) and test them on a robot

[sfoster1]

Some changes explicitly called out, but also we need to significantly rework the way transfers work. The code is a bit of a nightmare right now. We should make state more centralized and explicit, and we should take the time now to think about making multichannel transfers better and how to handle 384-plates. This may include changing the labware spec.

One thing we can do for testing that I think will help a deal is

• Change the robot singleton to write out all "atomic" ops (aspirate, dispense, move, mix, blowout, airgap, pick up, drop tip) to a log in a machine parseable fashion
• Make our contexts do the same
• write some protocols that do transfers and execute them on both and make sure they match (except for any behavioral bugs we find and fix).

[codecov]

Codecov Report

Merging #2713 into edge will increase coverage by 2.33%.
The diff coverage is 80.37%.

@@            Coverage Diff             @@
##             edge    #2713      +/-   ##
==========================================
+ Coverage   51.68%   54.01%   +2.33%     
==========================================
  Files         662      667       +5     
  Lines       18922    20471    +1549     
==========================================
+ Hits         9779    11058    +1279     
- Misses       9143     9413     +270

Impacted Files	Coverage Δ
api/src/opentrons/types.py	87.5% <100%> (+1.38%)	⬆️
api/src/opentrons/protocol_api/labware.py	93.89% <100%> (ø)	⬆️
api/src/opentrons/protocol_api/contexts.py	83.89% <67.88%> (-1.49%)	⬇️
api/src/opentrons/api/session.py	88.85% <81.81%> (-0.54%)	⬇️
api/src/opentrons/protocol_api/transfers.py	83.39% <83.39%> (ø)
api/src/opentrons/commands/commands.py	96.93% <95.45%> (+0.93%)	⬆️
protocol-designer/src/steplist/utils/orderWells.js	85% <0%> (-15%)	⬇️
app/src/protocol/protocol-data.js	83.33% <0%> (-10%)	⬇️
...steplist/formLevel/stepFormToArgs/mixFormToArgs.js	3.44% <0%> (-1.32%)	⬇️
app/src/protocol/index.js	97.91% <0%> (-0.27%)	⬇️
... and 25 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5b375c...8378cc4. Read the comment docs.

[sfoster1]

Some nitpicks around documentation, old code, and some parameter types and verification but on the whole this looks really good!

[sfoster1]

The comment calls out location as being able to take either a well or position relative to a well (so, a Location), and I think that's how it should be. We should change location here to take basically the same thing as aspirate.

[sfoster1]

Unfortunately this docstring isn't formatted for sphinx. The generated documentation therefore doesn't really work.

[sfoster1]

Documenting extensive **kwargs parameters appears to be a thing for which there's not really a specific best practice in Sphinx, but the way this docstring is defined doesn't render very well.

I'd suggest we do one of

• Continue the way this works, with the params documented nested under the kwargs entry in the params list
• Explicitly reference TransferOptions if possible (may not be possible since some kwargs are interpreted here)
• Add another section called Additional Options or something and call out individual options there, basically hand formatting it to match the look of the autodoc-generated rst (per this SO answer

Whatever we go with, we should probably use a raw rst list since the way it is right now actually has all the kwarg keys on the same indent level as the kwargs callout, doesn't have them quoted, and has a line break in between the key, the type, and the description.

[sfoster1]

This function is private and the module is private too, but still the docstring should be informative and well formatted. It looks like some stuff here is outdated and some stuff isn't - we should just take a pass and make sure it's accurate and understandable by other devs, and format it as rst in case we ever generate docs from it.

[sanni-t]

Could you point to which stuff looks outdated? Maybe I am missing something or I made a mistake in writing a method

[sfoster1]

touch_tip on troughs doesn't quite work right because of another length vs width issue.

We have pretty consistently defined overallLength to be the longer of the outer dimensions and overallWidth to be the shorter of the outer dimensions of a labware, which means length=x span and width=y span. The trough's well definitions follow this also. Unfortunately, Well._from_center_cartesian doesn't, and thinks that length corresponds with y and width corresponds with x. So we'll need to change those back.

[sfoster1]

Mix, blowout, and airgap seem to work though.

[sfoster1]

Docs look good, functionality looks good. Touch tip on troughs is definitely weird but I think that's out of scope because it's certainly touching all 4 walls correctly, the question is just "is that actually a useful thing to do".

[sanni-t]

Found a bug in transfer() because of which new_tip and disposal_vol values were not getting assigned. Pushed a change to fix that. Also added a test which checks the transfer() kwargs

[btmorr]

Are these TODOs resolved?

[btmorr]

Is this TODO still relevant?

[btmorr]

🎈

[mcous]

🛬