feat(api): Implement `define_liquid` and `load_liquid` in PAPI #11920

TamarZanzouri · 2022-12-21T21:12:21Z

Overview

closes https://opentrons.atlassian.net/browse/RLIQ-7.
Add a define_liquid and load_liquid in PAPI.

Test Plan

Test against an OT-2\OT-3 with core ff on.
Upload a python protocol to test against PAPI.

from opentrons import protocol_api, types

metadata = {
    "apiLevel": "2.13",
}

def run(ctx: protocol_api.ProtocolContext) -> None:
    labware = ctx.load_labware("nest_96_wellplate_100ul_pcr_full_skirt", 5)
    liquid = ctx.define_liquid(name="water", description="", display_color="#fff")
    well = labware.wells()[0]
    well.load_liquid(liquid=liquid, volume=20.0)

Make sure the robot analysis passes.
Test with ff off. Make sure you are getting an unsupported exception.

Changelog

added define_liquid to protocol_context and to protocol core's implementation.
added load_liquid to well_context and to well core's implementation.
added load_liquid to sync_client.
added add_liquid to sync_client.

Review requests

did I miss anything?
are test sufficient?

Risk assessment

Low. added a new end points in the PAPI to define_liquid and load_liquid.

… in papi

api/src/opentrons/protocol_api/core/protocol_api/well.py

api/src/opentrons/protocol_api/labware.py

codecov · 2023-01-13T16:31:30Z

Codecov Report

Merging #11920 (3194f3a) into edge (7d7839c) will not change coverage.
The diff coverage is 80.00%.

@@           Coverage Diff           @@
##             edge   #11920   +/-   ##
=======================================
  Coverage   74.00%   74.00%           
=======================================
  Files        2193     2193           
  Lines       60665    60665           
  Branches     6418     6418           
=======================================
  Hits        44894    44894           
  Misses      14236    14236           
  Partials     1535     1535

Flag	Coverage Δ
notify-server	`89.13% <ø> (ø)`

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

Impacted Files	Coverage Δ
api/src/opentrons/protocol_api/__init__.py	`100.00% <ø> (ø)`
api/src/opentrons/protocol_api/protocol_context.py	`91.37% <62.50%> (ø)`
api/src/opentrons/protocol_api/labware.py	`90.60% <66.66%> (ø)`
...c/opentrons/protocol_engine/clients/sync_client.py	`100.00% <100.00%> (ø)`
...i/src/opentrons/protocol_engine/protocol_engine.py	`100.00% <100.00%> (ø)`
...i/src/opentrons/protocol_runner/protocol_runner.py	`100.00% <100.00%> (ø)`
... and 1 more

api/tests/opentrons/protocol_api/core/engine/test_well_core.py

api/src/opentrons/protocol_api/labware.py

TamarZanzouri · 2023-01-24T17:43:27Z

Tested on OT-2 with core ff on. tested json/python protocols. works as expected.

ecormany

Couple comments on docstrings including 1 typo.

api/src/opentrons/protocol_api/protocol_context.py

api/src/opentrons/protocol_api/core/legacy/legacy_protocol_core.py

api/src/opentrons/protocol_api/core/legacy/legacy_well_core.py

api/src/opentrons/protocol_api/labware.py

SyntaxColoring · 2023-01-24T20:58:16Z

api/src/opentrons/protocol_engine/protocol_engine.py

+ def add_liquid(
+ self,
+ name: str,
+ color: Optional[str] = None,
+ description: str = "",
+ id: Optional[str] = None,
+ ) -> Liquid:
 """Add a liquid to the state for subsequent liquid loads."""
- self._action_dispatcher.dispatch(AddLiquidAction(liquid=liquid))
+ if id is None:
+ id = self._model_utils.generate_id()
+ self._action_dispatcher.dispatch(
+ AddLiquidAction(id=id, name=name, color=color, description=description)
+ )
+ return Liquid(
+ id=id,
+ displayName=name,
+ description=description,
+ displayColor=HexColor(__root__=color) if color else None,
+ )


Could you explain this change?

It looks like we're constructing a Liquid in the exact same way here, in LiquidStore, and in ProtocolRunner. Why shouldn't AddLiquidAction continue to take an already-constructed Liquid?

api/src/opentrons/protocol_api/_liquid.py

api/src/opentrons/protocol_api/core/engine/protocol.py

api/src/opentrons/protocol_engine/protocol_engine.py

api/src/opentrons/protocol_engine/state/liquids.py

api/src/opentrons/protocol_engine/protocol_engine.py

api/tests/opentrons/protocol_api/core/engine/test_protocol_core.py

Co-authored-by: Mike Cousins <[email protected]>

Co-authored-by: Ed Cormany <[email protected]>

ecormany

Just a couple docstring typos.

api/src/opentrons/protocol_api/_liquid.py

api/src/opentrons/protocol_api/protocol_context.py

Co-authored-by: Ed Cormany <[email protected]>

Docstrings all fixed now!

TamarZanzouri · 2023-01-27T17:52:03Z

Tested on dev server with core ff on, python protocols and json protocols. works as expected!

mcous

LGTM

TamarZanzouri added 17 commits December 15, 2022 15:56

create_liquid AbstractProtocol and public python api

f9d250b

WIP - api version error in legacy. test + started engine core

5121a6d

add_liquid in sync client

a971a56

renamed to add_liquid and changed test to use matchers

a33101c

added test to public context

e063f7d

added class for returned python liquid

db295af

changed type to dataclass and fixed failing test

1460511

moved LoadedLiquid class to core/protocol. added initial test to load…

acce45c

… in papi

added abstract method load_liquid and class declarations

97022e2

wip started implementing engine/protocol load_liquid

6aff684

engine/protocol impl

8a9264f

impl for sync client. changing type to LabwareCore

9fc83f1

WIP change Well to WellCoreType

56e3057

changed Well to WellCore

90e90a6

changed dict to str, float

736cbad

WIP load liquid into well

06bb6a7

finished engine core, moved type to new file and moved and fixed tests

1115693

TamarZanzouri commented Jan 13, 2023

View reviewed changes

api/src/opentrons/protocol_api/core/protocol_api/well.py Outdated Show resolved Hide resolved

TamarZanzouri commented Jan 13, 2023

View reviewed changes

api/src/opentrons/protocol_api/labware.py Outdated Show resolved Hide resolved

TamarZanzouri force-pushed the RLIQ-7-papi-load-liquid branch from 8d814dd to 1115693 Compare January 13, 2023 16:34

Merge branch 'edge' into RLIQ-7-papi-load-liquid

1b99e28

TamarZanzouri commented Jan 13, 2023

View reviewed changes

api/tests/opentrons/protocol_api/core/engine/test_well_core.py Outdated Show resolved Hide resolved

Update api/tests/opentrons/protocol_api/core/engine/test_well_core.py

2b8908b

TamarZanzouri added the robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience). label Jan 13, 2023

TamarZanzouri changed the title ~~Rliq 7 papi load liquid~~ feature(api): Implement add_liquid and load_liquid in PAPI Jan 13, 2023

mcous changed the title ~~feature(api): Implement add_liquid and load_liquid in PAPI~~ feat(api): Implement add_liquid and load_liquid in PAPI Jan 13, 2023

TamarZanzouri commented Jan 13, 2023

View reviewed changes

api/src/opentrons/protocol_api/labware.py Outdated Show resolved Hide resolved

TamarZanzouri and others added 2 commits January 13, 2023 13:12

Update api/src/opentrons/protocol_api/labware.py

66941c7

change liquid color to optional

1e16b7f

ecormany reviewed Jan 24, 2023

View reviewed changes

SyntaxColoring reviewed Jan 24, 2023

View reviewed changes

mcous suggested changes Jan 24, 2023

View reviewed changes

TamarZanzouri and others added 10 commits January 24, 2023 16:44

changed back the AddLiquidAction to contain the Liquid

ccb8bb9

Update api/src/opentrons/protocol_api/core/engine/protocol.py

ef03f48

Co-authored-by: Mike Cousins <[email protected]>

Update api/src/opentrons/protocol_api/core/engine/protocol.py

73ecd22

Co-authored-by: Mike Cousins <[email protected]>

added docstring and changed var name

ccdab01

Merge branch 'edge' into RLIQ-7-papi-load-liquid

e31f7c4

removed default values and changed type to HexColor

eb99c4f

Update api/src/opentrons/protocol_api/protocol_context.py

f3e6c18

Co-authored-by: Ed Cormany <[email protected]>

pr feedback

8325d69

pr fixes

b3573c2

cleanup

a314749

TamarZanzouri requested review from ecormany, mcous and SyntaxColoring January 25, 2023 21:08

ecormany previously requested changes Jan 25, 2023

View reviewed changes

api/src/opentrons/protocol_api/_liquid.py Outdated Show resolved Hide resolved

api/src/opentrons/protocol_api/protocol_context.py Outdated Show resolved Hide resolved

TamarZanzouri and others added 2 commits January 26, 2023 10:19

Update api/src/opentrons/protocol_api/_liquid.py

433ed1e

Co-authored-by: Ed Cormany <[email protected]>

Update api/src/opentrons/protocol_api/protocol_context.py

f2622d8

Co-authored-by: Ed Cormany <[email protected]>

TamarZanzouri requested a review from ecormany January 26, 2023 15:48

added versioning.rst

c5a4405

mcous changed the title ~~feat(api): Implement add_liquid and load_liquid in PAPI~~ feat(api): Implement define_liquid and load_liquid in PAPI Jan 27, 2023

moved default values to the PE and fixed versioning

3194f3a

mcous approved these changes Jan 27, 2023

View reviewed changes

TamarZanzouri merged commit 643f0c4 into edge Jan 27, 2023

TamarZanzouri deleted the RLIQ-7-papi-load-liquid branch January 27, 2023 19:33

SyntaxColoring mentioned this pull request Feb 1, 2023

chore(release): Merge hotfix_6.2.1@a63f4fcf into edge (merge 3) #12086

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): Implement `define_liquid` and `load_liquid` in PAPI #11920

feat(api): Implement `define_liquid` and `load_liquid` in PAPI #11920

TamarZanzouri commented Dec 21, 2022 •

edited

Loading

codecov bot commented Jan 13, 2023 •

edited

Loading

TamarZanzouri commented Jan 24, 2023

ecormany left a comment

SyntaxColoring Jan 24, 2023

ecormany left a comment

TamarZanzouri commented Jan 27, 2023

mcous left a comment

feat(api): Implement define_liquid and load_liquid in PAPI #11920

feat(api): Implement define_liquid and load_liquid in PAPI #11920

Conversation

TamarZanzouri commented Dec 21, 2022 • edited Loading

Overview

Test Plan

Changelog

Review requests

Risk assessment

codecov bot commented Jan 13, 2023 • edited Loading

Codecov Report

TamarZanzouri commented Jan 24, 2023

ecormany left a comment

Choose a reason for hiding this comment

SyntaxColoring Jan 24, 2023

Choose a reason for hiding this comment

ecormany left a comment

Choose a reason for hiding this comment

TamarZanzouri commented Jan 27, 2023

mcous left a comment

Choose a reason for hiding this comment

feat(api): Implement `define_liquid` and `load_liquid` in PAPI #11920

feat(api): Implement `define_liquid` and `load_liquid` in PAPI #11920

TamarZanzouri commented Dec 21, 2022 •

edited

Loading

codecov bot commented Jan 13, 2023 •

edited

Loading