feat(api): implement labware.set_offset in 2.18 #14940

sfoster1 · 2024-04-17T20:20:43Z

The python protocol api's labware.set_offset() command lets you set a labware position check offset programmatically. This is useful in protocols run outside the app (i.e. through Jupyter, through opentrons_execute).

When we moved to the protocol engine, we didn't support running those protocols outside the app, and therefore we didn't implement this method on those new API versions.

Now, we allow that again, so implement that method again.

This relies on dispatching AddLabwareOffset actions at arbitrary times, because it will set an offset for the location in which the labware is currently present. This allows the user to set offsets without having to wrangle with the engine's internal definitions of where something is, at the cost of the user having to spread their offset calls around their protocol if they have protocols that load things in one place and later move them to another.

Warning

This won't work the way you think it will right now. Labware offsets are applied to labware instances by the LoadLabware and MoveLabware commands. So if you do an AddLabwareOffsetAction after a LoadLabware, the offset will never apply to the labware. We need to add a new ReloadLabware command, but I'm going to do that in a separate PR and then put this PR on top of that one.
Fully implemented yay

Closes RSQ-29

To Leave Draft

tests
general agreement that this is the right path
autogen doc updates (i.e. docstrings for set_offset
rebase on top of PR that adds ReloadLabware
also dispatch reload labware

DerekMaggio · 2024-04-17T20:25:44Z

How are we going to communicate to users where/when they need to call set_offset?
Seems to me that documenting that might be harder than documenting how to use the engines internal definitions of where something is.

Don't each one of the internal definitions of the locations correspond to, "an offset we calculate in LPC"?

Can you maybe provide an example of what users would have to define if we went the other route?

sfoster1 · 2024-04-17T20:37:21Z

How are we going to communicate to users where/when they need to call set_offset? Seems to me that documenting that might be harder than documenting how to use the engines internal definitions of where something is.
I really disagree with this. I think it's very documentable to say "use set_offset by calling it right after you move a labware"
Don't each one of the internal definitions of the locations correspond to, "an offset we calculate in LPC"?
yes, but it's annoying (see below) to actually wrangle the various function calls, unions, isinstance checks, and type imports to do it correctly.
Can you maybe provide an example of what users would have to define if we went the other route?

if you went the other way, they'd have to do this: https://github.com/Opentrons/opentrons/pull/14940/files#diff-6f733f7119a96d280a77b7a2c54d99087fa07c7759299c4ce029c48ec0c1c668R103-R154 or we'd have to provide some function to look it up

TamarZanzouri · 2024-04-17T20:40:41Z

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

@@ -91,10 +100,65 @@ def get_parameters(self) -> LabwareParametersDict:
 def get_quirks(self) -> List[str]:
 return self._definition.parameters.quirks or []

+ def _get_offset_location(self) -> LabwareOffsetLocation:
+ slot = self.get_deck_slot()
+ if None is slot:


nip but can we do this instead?

Suggested change

if None is slot:

if slot is None:

TamarZanzouri

LGTM! just a few comments

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

DerekMaggio · 2024-04-17T21:03:29Z

@sfoster1, your response to my earlier comment helped me understand.
I thought users would get confused about keeping track of where the labware was at. But they have to do that regardless.

My mistake, you're correct, the downside of having to do all those imports and describing to the user how to use said imports, is much more difficult than just telling them to call set offset.

This looks good to me so far

SyntaxColoring

Cool, TY!

I think we'll need to document on the public set_offset() method that identical labware in the same location will share the same offset. This is the opposite of what the PAPIv≤2.13 behavior was, and what is currently documented.

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

sfoster1 · 2024-04-17T21:14:47Z

Cool, TY!

I think we'll need to document on the public set_offset() method that identical labware in the same location will share the same offset. This is the opposite of what the PAPIv≤2.13 behavior was, and what is currently documented.

Right, definitely - it's now adding a labware offset. Can do.

sfoster1 · 2024-04-19T14:51:38Z

@ecormany Would love your opinion on the documentation strategy here

sfoster1 · 2024-04-19T21:34:07Z

Cool, TY!

I think we'll need to document on the public set_offset() method that identical labware in the same location will share the same offset. This is the opposite of what the PAPIv≤2.13 behavior was, and what is currently documented.

#14967

Adds a new command ReloadLabware, which allows dispatchers to change all the details of a loaded labware except for the location. This is primarily intended to allow getting a new labware offset that was not added to the engine by the time this labware was loaded (though it can technically do more, for symmetry). This doesn't really change a whole lot of behavior and is well-supported with testing. It's a prerequisite for #14940 Closes RSQ-29

The python protocol api's labware.set_offset() command lets you set a labware position check offset programmatically. This is useful in protocols run outside the app (i.e. through Jupyter, through opentrons_execute). When we moved to the protocol engine, we didn't support running those protocols outside the app, and therefore we didn't implement this method on those new API versions. Now, we allow that again, so implement that method again. This relies on dispatching AddLabwareOffset actions at arbitrary times, because it will set an offset for the location in which the labware is _currently_ present. This allows the user to set offsets without having to wrangle with the engine's internal definitions of where something is, at the cost of the user having to spread their offset calls around their protocol if they have protocols that load things in one place and later move them to another. Closes RSQ-29

or at least tests for the protocol api and protocol api core part. also a little refactor to make it easier to specify version ranges

SyntaxColoring

One note about requested vs. connected module model, but this LGTM other than that. Thank you!

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

api/tests/opentrons/protocol_engine/state/test_geometry_view.py

api/tests/opentrons/protocol_api/__init__.py

SyntaxColoring

Hooray

@sfoster1

# Overview Documents [updated behavior](#14940) for `Labware.set_offset()` in Python API version 2.18. Addresses RTC-234 # Test Plan Sandbox: - [API reference entry](http:https://sandbox.docs.opentrons.com/docs-set_offset-updates/v2/new_protocol_api.html#opentrons.protocol_api.Labware.set_offset) - [new section](http:https://sandbox.docs.opentrons.com/docs-set_offset-updates/v2/new_advanced_running.html#labware-offset-behavior) of Advanced Control # Changelog - adapted new docstring draft written by @sfoster1 - adapt sample code and move it to Advanced Control # Review requests have we accurately captured the three phases of behavior (2.12–13, 14–17, 18+)? # Risk assessment none

@sfoster1

# Overview Documents [updated behavior](#14940) for `Labware.set_offset()` in Python API version 2.18. Addresses RTC-234 # Test Plan Sandbox: - [API reference entry](http:https://sandbox.docs.opentrons.com/docs-set_offset-updates/v2/new_protocol_api.html#opentrons.protocol_api.Labware.set_offset) - [new section](http:https://sandbox.docs.opentrons.com/docs-set_offset-updates/v2/new_advanced_running.html#labware-offset-behavior) of Advanced Control # Changelog - adapted new docstring draft written by @sfoster1 - adapt sample code and move it to Advanced Control # Review requests have we accurately captured the three phases of behavior (2.12–13, 14–17, 18+)? # Risk assessment none

Adds a new command ReloadLabware, which allows dispatchers to change all the details of a loaded labware except for the location. This is primarily intended to allow getting a new labware offset that was not added to the engine by the time this labware was loaded (though it can technically do more, for symmetry). This doesn't really change a whole lot of behavior and is well-supported with testing. It's a prerequisite for #14940 Closes RSQ-29

The python protocol api's labware.set_offset() command lets you set a labware position check offset programmatically. This is useful in protocols run outside the app (i.e. through Jupyter, through opentrons_execute). When we moved to the protocol engine, we didn't support running those protocols outside the app, and therefore we didn't implement this method on those new API versions. Now, we allow that again, so implement that method again. This relies on dispatching AddLabwareOffset actions at arbitrary times, because it will set an offset for the location in which the labware is _currently_ present. This allows the user to set offsets without having to wrangle with the engine's internal definitions of where something is, at the cost of the user having to spread their offset calls around their protocol if they have protocols that load things in one place and later move them to another. ## Warning ~This won't work the way you think it will right now. Labware offsets are applied to labware instances by the `LoadLabware` and `MoveLabware` commands. So if you do an `AddLabwareOffsetAction` after a `LoadLabware`, the offset will never apply to the labware. We need to add a new `ReloadLabware` command, but I'm going to do that in a separate PR and then put this PR on top of that one.~ Fully implemented yay Closes RSQ-29

sfoster1 requested review from SyntaxColoring and a team April 17, 2024 20:20

TamarZanzouri reviewed Apr 17, 2024

View reviewed changes

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

SyntaxColoring reviewed Apr 17, 2024

View reviewed changes

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

sfoster1 marked this pull request as ready for review April 19, 2024 14:51

sfoster1 requested a review from a team as a code owner April 19, 2024 14:51

sfoster1 requested a review from ecormany April 19, 2024 14:51

sfoster1 marked this pull request as draft April 19, 2024 14:53

sfoster1 mentioned this pull request Apr 19, 2024

feat(api): add a reload-labware command #14963

Merged

1 task

sfoster1 force-pushed the RSQ-29-add-set-labware-offset-back branch from 07b8690 to 3db0724 Compare April 19, 2024 18:11

sfoster1 changed the base branch from edge to rsq-29-add-reload-labware April 19, 2024 18:12

sfoster1 force-pushed the RSQ-29-add-set-labware-offset-back branch from 3db0724 to 97d18eb Compare April 19, 2024 18:22

sfoster1 marked this pull request as ready for review April 19, 2024 18:39

sfoster1 force-pushed the RSQ-29-add-set-labware-offset-back branch from 3c9fbce to b1d55c5 Compare April 19, 2024 19:38

Base automatically changed from rsq-29-add-reload-labware to edge April 24, 2024 21:39

sfoster1 requested a review from a team as a code owner April 24, 2024 21:39

sfoster1 added 5 commits April 24, 2024 17:39

fixup: move get_offset_location to engine, add tests

8267106

or at least tests for the protocol api and protocol api core part. also a little refactor to make it easier to specify version ranges

alter error message

f6e3032

a simple (lol) new test

18e4390

finish up the positive tests

e12c9b9

sfoster1 added 4 commits April 24, 2024 17:39

check off deck labware

96fd9e4

update method docs

5daa389

actually dispatch a reload command

1bf0244

revert docstring changes since they'll happen later

c2b5327

sfoster1 force-pushed the RSQ-29-add-set-labware-offset-back branch from b1d55c5 to c2b5327 Compare April 24, 2024 21:45

SyntaxColoring reviewed Apr 25, 2024

View reviewed changes

wrong kind of model

5ccd24b

SyntaxColoring approved these changes Apr 25, 2024

View reviewed changes

sfoster1 merged commit cf9b6d2 into edge Apr 25, 2024
20 checks passed

sfoster1 deleted the RSQ-29-add-set-labware-offset-back branch April 25, 2024 16:54

sfoster1 mentioned this pull request Apr 25, 2024

test: add v2.18 smoke test protocol #14946

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): implement labware.set_offset in 2.18 #14940

feat(api): implement labware.set_offset in 2.18 #14940

sfoster1 commented Apr 17, 2024 •

edited

Loading

DerekMaggio commented Apr 17, 2024 •

edited

Loading

sfoster1 commented Apr 17, 2024

TamarZanzouri Apr 17, 2024

TamarZanzouri left a comment •

edited

Loading

DerekMaggio commented Apr 17, 2024

SyntaxColoring left a comment

sfoster1 commented Apr 17, 2024

sfoster1 commented Apr 19, 2024

sfoster1 commented Apr 19, 2024

SyntaxColoring left a comment

SyntaxColoring left a comment

feat(api): implement labware.set_offset in 2.18 #14940

feat(api): implement labware.set_offset in 2.18 #14940

Conversation

sfoster1 commented Apr 17, 2024 • edited Loading

Warning

To Leave Draft

DerekMaggio commented Apr 17, 2024 • edited Loading

sfoster1 commented Apr 17, 2024

TamarZanzouri Apr 17, 2024

Choose a reason for hiding this comment

TamarZanzouri left a comment • edited Loading

Choose a reason for hiding this comment

DerekMaggio commented Apr 17, 2024

SyntaxColoring left a comment

Choose a reason for hiding this comment

sfoster1 commented Apr 17, 2024

sfoster1 commented Apr 19, 2024

sfoster1 commented Apr 19, 2024

SyntaxColoring left a comment

Choose a reason for hiding this comment

SyntaxColoring left a comment

Choose a reason for hiding this comment

sfoster1 commented Apr 17, 2024 •

edited

Loading

DerekMaggio commented Apr 17, 2024 •

edited

Loading

TamarZanzouri left a comment •

edited

Loading