Skip to content
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): overhaul Labware docstrings #14401

Merged
merged 3 commits into from
Feb 13, 2024
Merged

docs(api): overhaul Labware docstrings #14401

merged 3 commits into from
Feb 13, 2024

Conversation

ecormany
Copy link
Contributor

@ecormany ecormany commented Jan 31, 2024

Overview

Freshen up the Labware class in the API Reference.

Addresses RTC-205.

Test Plan

Check the sandbox.

Changelog

  • General cleanup of docstrings.
  • Added one cross-reference target.

Review requests

Take a careful look at the simplifications I made in set_offset. I think we need to more fully address what the current state of that function is. The ticket referenced in the TODO just above the error handling logic has been closed. I think that function should be usable again, but I won't put the docs ahead of the API's actual capabilities.

Risk assessment

none, docs

Copy link

codecov bot commented Jan 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (62584b8) 68.24% compared to head (5bde389) 68.16%.
Report is 61 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #14401      +/-   ##
==========================================
- Coverage   68.24%   68.16%   -0.08%     
==========================================
  Files        2514     2514              
  Lines       71868    71854      -14     
  Branches     9176     9176              
==========================================
- Hits        49044    48981      -63     
- Misses      20657    20706      +49     
  Partials     2167     2167              
Flag Coverage Δ
g-code-testing 92.43% <ø> (-4.06%) ⬇️

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

Files Coverage Δ
api/src/opentrons/protocol_api/labware.py 91.34% <100.00%> (+0.76%) ⬆️

... and 5 files with indirect coverage changes

@ecormany ecormany added docs papi-v2 Python API V2 labels Feb 1, 2024
@ecormany ecormany marked this pull request as ready for review February 2, 2024 01:57
@ecormany ecormany requested a review from a team as a code owner February 2, 2024 01:57
@ecormany ecormany requested review from jwwojak and SyntaxColoring and removed request for a team February 2, 2024 01:57
Copy link
Contributor

@jwwojak jwwojak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple of comments. Overall, LGTM. Defer to Max for a follow on review/approval.

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

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay, thanks.

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

@jwwojak jwwojak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. No comments.

:shipit:

@ecormany ecormany merged commit 0a410c8 into edge Feb 13, 2024
22 checks passed
@ecormany ecormany deleted the docstrings-labware branch February 13, 2024 21:08
andySigler pushed a commit that referenced this pull request Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs papi-v2 Python API V2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants