-
Notifications
You must be signed in to change notification settings - Fork 179
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): Refactor instrument-context docstrings #13780
Conversation
Line 116, "millimiters" should be "millimeters".
Use "µL" instead of "uL".
ignore
Inconsistent punctuation for the text ":returns: This instance." Sometimes ended with a period, sometimes not. Changed to end all instances w/ a period.
Too many instances of "eg". Should be "e.g.," or "e.g."
Not rendering in a nice way. Might be exceeding character count per line.
Again, more line length testing.
Anchor to pipette_tips.rst, "Dropping a Tip" section. Anchor is .. _pipette-drop-tip:
Picking up and trying again. Ran Black. Output suggests this might be ok:
Also, resolved small text conflict around |
Line count doesn't change
yep, and the "API test/lint/deploy" tests are passing in CI ✅ |
Changing status from draft. |
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.
This is big, so I left lots of comments. Mostly small stuff on the edited parts. A few methods and parameters were passed over entirely and need a first round of edits.
@@ -1524,7 +1531,7 @@ def hw_pipette(self) -> PipetteDict: | |||
@property # type: ignore | |||
@requires_version(2, 0) | |||
def channels(self) -> int: | |||
"""The number of channels on the pipette.""" | |||
"""The number of channels on the pipette. See also, :ref:`new-pipette`.""" |
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.
add that it's 1, 8, or 96?
|
||
The Flex 1-Channel 50 µL and Flex 8-Channel 50 µL pipettes must operate in a low-volume mode | ||
to accurately dispense 1 µL of liquid. Low-volume mode can only be set by calling this function. | ||
"""Configure a pipette to handle a specific volume of liquid, measured in µL. Depending on the volume, the pipette will enter a unique pipetting mode. |
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.
not sure i like "unique" here. there's two modes, and that makes it sound like there's a lot more. stet or The pipette enters a volume mode depending on the volume provided.
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.
Revised using the recommended text.
|
||
For more information on available modes for certain pipettes, see :ref:`pipette-volume-modes`. | ||
The Flex 1-Channel 50 µL and Flex 8-Channel 50 µL pipettes must operate in a low-volume mode to accurately dispense 1 µL of liquid. Low-volume mode can | ||
only be set by calling ``configure_for_volume``. See also, :ref:`pipette-volume-modes`. |
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 like the demonstrative pronoun better. indicates "you are already here". i could see reading this and thinking "oh i have to look up another function"
Making batch commits before other changes. Will do a batch commit, pull locally, more revisions, and push again. Co-authored-by: Ed Cormany <[email protected]>
For text like "See, . . . " and "See also, . . . " let's get rid of the commas. Also, most, if not all, of these should be just "See" not "See also" because we're referring to stuff that directly supports the text.
Single quotes made code italics. Replace with ``code`` to get monospace. Remove * as a multiplication operator. Replace with "multiplied by". Fix code sample in dispense. Run Black check. Looks ok.
Incorporating reviewer comments. - aspirate - dispense - reset_tipracks - touch_tip - remove (s) - small adjustments Not finished yet. More to come.
- Replace (text only) "tiprack" with "tip rack" - Fix single quotes around code elements. Replace with double single quotes (`not this`, but ``this instead``). Rendered text in italics. - drop_tip revisions - mount revisions, add "left" and "right" - min_volume revision, add definition - configure_for_volume changes
Another instance of italics instead of monospace.
I've seen formatting issues in the labware class too, but that's out of scope for this PR, which covers the InstrumentContext. For example columns_by_name, code syntax using single quotes. |
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.
The substance is good; all that remains is pedantry! I've left many comments to make a record of why certain changes are necessary. I'll batch up the suggestions into a commit and then also do a final pass through to clean up line lengths. Then this will be ready to merge.
and run Black. All done! ✨ 🍰 ✨ 1 file reformatted.
Following up on #13780 with some additional improvements to docstrings within InstrumentContext.
Following up on #13780 with some additional improvements to docstrings within InstrumentContext.
Overview
This PR is for managing suggested revisions to the docstrings in
instrument_context.py
. This is descriptive, customer/user-facing text that appears in Protocols and Instruments section of the API v2 reference. JIRA issue is RTC-204.These are lightweight changes that revise text, grammar, style, punctuation. Changes also include anchors to longer, more expository API docs throughout because many of the docstrings are terse and/or brief. Linking to other docs helps provide readers with more context, information, code samples, and broader understanding of our API methods. In all cases, we've tried to change as little text as possible.
Excluded: any text related to warnings and errors (
raises
,runtime
,type
, etc.) or anything that is a combination of static text and dynamic code. Just revisions to main definitions text only.Test Plan
Sandbox: http://sandbox.docs.opentrons.com/instrument-context-docstrings/v2/new_protocol_api.html#
Testing consisted of making text revisions, pushing to the working branch, and reviewing text results in the sandbox.
Changelog
Definition text changes throughout. Things like:
Review requests
A lot of this is small and intricate. Any second (or third or n) set of eyes reviewing helps.
Risk assessment
Low, docs/text change only, at least I hope I've only changed text and not working code. heh.