Skip to content

Conversation

@y3rsh
Copy link
Member

@y3rsh y3rsh commented Nov 21, 2025

Soooooo

review for #20121

  • we don't need the docs build anymore because it is built by /docs right???
  • all the changes in /api are 🪄 AI magic 🪄 I did not review but the sphinx build make -C api docs work... does that even matter? Just wanted to see what it did to fix that.
    • except api/src/opentrons/protocol_api/protocol_context.py which seems like a legitimate correction
  • changes in pyproject.toml, Makefile, and uv.lock help us have a stable dev/ci environment with the editable installs included.

Copy link
Contributor

@ecormany ecormany left a comment

Choose a reason for hiding this comment

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

This all seems fine, but I would definitely like another engineer to take a look too!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, correct to get rid of this one. The only casualty at this point is automatic build of the hardware API docs. I believe @SyntaxColoring advocated for us to keep generating them back when I removed the v1 and OT-One docs from CI. If we still want them on the web, we can and should bring them to mkdocs as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

This one is over my head. What does this file do for us?

Copy link
Member Author

Choose a reason for hiding this comment

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

ya this is the magic it took to get the build to work, ignore it.

Comment on lines +475 to +484
("py:class", r"optional"),
("py:class", r"ModuleContext"),
("py:class", r"DeckLocation"),
("py:class", r"MeniscusTrackingTarget"),
("py:class", r"NozzleLayout"),
]

nitpick_ignore = [
("py:exc", "LiquidClassDefinitionDoesNotExist"),
("py:exc", "UnexpectedTipRemovalError"),
Copy link
Contributor

Choose a reason for hiding this comment

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

We need these to get in edge for the Sphinx docs to build right now — maybe should do as a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

For sure what you think is best.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤷‍♂️ to these small changes. If we want to tear out the old Sphinx PAPI v2 docs, we should delete the whole api/docs/v2/ directory.

Comment on lines +18 to +25
"opentrons",
"opentrons-shared-data",
]

[tool.uv.sources]
mkdocs-parent-css-plugin = { path = "mkdocs-parent-css-plugin", editable = true }
opentrons-shared-data = { path = "../shared-data", editable = true }
opentrons = { path = "../api", editable = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

This is what got me into trouble last week…uv sync kept failing with exactly this setup (or maybe not quite exactly…does order matter in listing dependencies? i had them alphabetical). But this seems to work, so let's try it.

Returns:
The loaded and initialized module: a
ModuleTypes: The loaded and initialized module, which may be a
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be a problem for mkdocs rendering, so this is effectively a docs change.

Image

Copy link
Member Author

@y3rsh y3rsh Nov 25, 2025

Choose a reason for hiding this comment

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

Ya take or leave, just noticed the type did not match the code.

@ecormany
Copy link
Contributor

So I think if all we want to do is have make -C api docs work, then the simplest thing to do is to narrow the scope of what that action does. i.e.

.PHONY: docs
docs: docs/dist/v2 docs/dist/hardware

becomes

.PHONY: docs
docs: docs/dist/hardware

What we do with that hardware docs output is an open question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants