Add python stubs to built wheels

[chadrik]

Adds support for IDE type completion and static type analysis

This PR implements the same workflow as the one I recently added to OpenImageIO: AcademySoftwareFoundation/OpenImageIO#4692

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: chadrik / name: Chad Dombrova (d680401, 27b8751)

[chadrik]

@zachlewis I'm still testing this, but just a heads up that this is coming.

[carolalynn]

Hi Chad, thanks for this! Let us know when you're ready for CI tests, and I'll kick them off.

[chadrik]

Hi Chad, thanks for this! Let us know when you're ready for CI tests, and I'll kick them off.

I'm ready!

[chadrik]

This is in pretty good shape.

One concern is that the tests demonstrate that certain functions accept None, but the pybind11 docstring signatures don't give us any indications of this, so the stubs are incorrect. For example, LegacyViewingPipeline.setDisplayViewTransform() and MatrixTransform.setTransform(). Would it be possible to use std::optional within the bindings to indicate which ones accept None?

[chadrik]

@zachlewis I added instructions on how to build the stubs without using docker

[chadrik]

This is ready for a new round of CI and (hopefully) final review

[chadrik]

Good news! The CI properly alerted me to the fact that the stubs needed to be rebuilt due to changes recently pushed to main. Rebased and rebuilt.

[chadrik]

This is ready for another run of the CI.

[zachlewis]

Hey @chadrik -- so, we discussed this PR at the most recent TSC meeting, and I think we'd like to automate the stub generation entirely in CI, as opposed to imposing additional complexity on contributor workflows. I don't have a clear idea of exactly what that might look like, but we're going to have a closer look at this over the next few weeks to see if we can arrive at a solution. Thanks for your patience!

[chadrik]

The linux jobs with DOC=ON are failing because the dotted path to the documented objects is different than expected:

[INFO] frozendoc: -.. py:function:: FixedFunctionStyleFromString(str: str) -> PyOpenColorIO.PyOpenColorIO.FixedFunctionStyle
[INFO] frozendoc: +.. py:function:: FixedFunctionStyleFromString(str: str) -> PyOpenColorIO.FixedFunctionStyle

I don't think these errors have anything to do with my changes. I thought maybe there was a chance that the existence of the .pyi files in the new wheels affected the docs, but there are two strong bits of evidence that suggestion otherwise:

1. the doc builds succeed on other platforms
2. the wheels that I'm building don't even have the stubs in them yet (due to a bug)

Any idea what might be causing these errors?

[zachlewis]

Any idea what might be causing these errors?

We're looking into it... FWIW, we're experiencing this elsewhere as well, so, as you've inferred, these failures aren't directly related to anything in this PR.

This PR did inspire a conversation in our TSC meeting about revisiting our whole documentation-building process, since our "frozen-docs" workflow, while fancy, imposes the sort of complexity we'd like to automate away with CI; to me, building python stubs, building documentation, and clang-formatting are similar, in that they're the sort of tasks it would be great to automate either just before or just after merging a PR. I known @lgritz is interested in this as well, having a github action perform clang-formatting and automatically submitting a PR for any delta, instead of blocking a merge...

[chadrik]

Hey @chadrik -- so, we discussed this PR at the most recent TSC meeting, and I think we'd like to automate the stub generation entirely in CI, as opposed to imposing additional complexity on contributor workflows. I don't have a clear idea of exactly what that might look like, but we're going to have a closer look at this over the next few weeks to see if we can arrive at a solution. Thanks for your patience!

Is the idea to push the changes as a commit to the branch, so that they can be reviewed as part of the MR?

[chadrik]

Is the idea to push the changes as a commit to the branch, so that they can be reviewed as part of the MR?

If so, this may offer a simple solution: https://github.com/stefanzweifel/git-auto-commit-action

[chadrik]

I made some progress on an automated-commit solution: AcademySoftwareFoundation/OpenImageIO#4802 (comment)

That said, it would be great if we could separate these two concerns: is merging this contingent upon solving the auto-commit problem?

[zachlewis]

Thanks Chad, I appreciate your continued help and patience with this...

That said, it would be great if we could separate these two concerns: is merging this contingent upon solving the auto-commit problem?

I think it may be contingent; or at least partially contingent. Our primary concern is with imposing additional complexity on the contribution process (i.e., for contributors); and if we can find a way to avoid imposing complexity on the maintenance process (i.e., stuff we have to manually do before and after a release), even better.

Put it this way -- solving the auto-commit problem would kill a few birds with one stone; but I agree, it's a separate concern.

Maybe we can separate out the CI / GHA stuff, and reframe this PR as a process that should take place before release (like the frozen-docs stuff), as opposed to a process that must take place before merging PRs? And then we can handle the automation implementation elsewhere?

[chadrik]

Maybe we can separate out the CI / GHA stuff, and reframe this PR as a process that should take place before release (like the frozen-docs stuff), as opposed to a process that must take place before merging PRs?

I can look into converting the job failure into a warning annotation:

In theory, the annotation can describe how to fix the problem, and make reviewers aware, without making the job failure a blocking issue.