-
Notifications
You must be signed in to change notification settings - Fork 178
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
docs(api): new behavior for Labware.set_offset()
in PAPI 2.18
#14967
Conversation
Labware.set_offset()
in PAPI 2.18
api/docs/v2/new_advanced_running.rst
Outdated
|
||
The effects of ``set_offset()`` vary depending on the API level of your protocol. | ||
|
||
.. list-table:: |
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.
Great use of a table here.
- In API level 2.13 and earlier, the offset applies only to the labware | ||
instance that ``set_offset()`` is called on. | ||
- In API levels 2.14–2.17, ``set_offset()`` is not available, and the API | ||
raises an error. | ||
- In API level 2.18 and newer, the offset applies to any labware of the | ||
same type, in the same on-deck location. Labware type is defined as the | ||
combination of the ``loadName``, ``namespace``, and ``version`` parameters | ||
of :py:meth:`~.ProtocolContext.load_labware`. | ||
See :ref:`labware-offset-behavior` for examples. |
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.
I wonder if a table would work here, similar to above. You might get rid of some repetition via the column headers:
Col 1 header: API Level
Col 2 header: Description
Eliminates the need to repeat "In API level x.x ...". In the first column, just write "API 2.13 and earlier"
In the first and 3rd bullets, the text uses "earlier" and "newer" respectively. It might be stronger or more clear to use "older" and "newer." Something about "earlier" sort of threw me off. I had to think for a moment, which probably explains the burning smell.
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.
I don't think we can embed a table in a docstring, although I'll give it a try.
You're right about the "earlier/newer" mismatch. Our standard is "earlier/later", and I'll update it.
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.
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.
Very nice, clear. Left a few comments for your consideration.
# 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://sandbox.docs.opentrons.com/docs-set_offset-updates/v2/new_protocol_api.html#opentrons.protocol_api.Labware.set_offset) - [new section](http://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
Overview
Documents updated behavior for
Labware.set_offset()
in Python API version 2.18.Addresses RTC-234
Test Plan
Sandbox:
Changelog
Review requests
have we accurately captured the three phases of behavior (2.12–13, 14–17, 18+)?
Risk assessment
none