Generate and ship committed PEP 561 type stubs for the Python extension - #912
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces PEP 561 type stub generation using pybind11-stubgen during the CMake build process, updates build-system requirements in pyproject.toml, and renames the reserved keyword argument 'in' to 'Tin' in UniTensor's put_block and put_block_ bindings while maintaining backward compatibility with deprecation warnings. Feedback on these changes suggests adding a CMake option to disable stub generation to support cross-compilation and prevent installation failures. Additionally, the reviewer recommends explicitly handling potential conflicts in put_block and put_block_ when both 'in' and 'Tin' keyword arguments are provided simultaneously.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #912 +/- ##
==========================================
+ Coverage 29.87% 30.04% +0.16%
==========================================
Files 240 240
Lines 35425 35460 +35
Branches 14729 14752 +23
==========================================
+ Hits 10584 10653 +69
+ Misses 17593 17520 -73
- Partials 7248 7287 +39
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
c7473d7 to
6700547
Compare
IvanaGyro
changed the title
bffc010 to
c1e1f45
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1e1f453a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 725a8ddd4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The C++ Network::construct declares outrk with a default value (= 0, see
include/Network.hpp), but the pybind11 binding bound it with a bare
py::arg("outrk") and no default. This had two consequences:
- Python callers were forced to pass outrk even though the C++ API treats
it as optional.
- A required parameter following the defaulted outlabel produces a function
signature with a non-default argument after a default one, which is
invalid Python. Tools that materialise the signature (e.g. type-stub
generators) emit a syntactically invalid result for this overload.
Bind outrk with its C++ default (= (cytnx_int64)0) so the Python signature
matches the C++ API and is a well-formed Python signature.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…egacy keyword
put_block and put_block_ bound their leading Tensor argument as py::arg("in").
'in' is a Python reserved word, so it could never be passed as put_block(in=...)
(a SyntaxError) and any materialised signature naming it 'in' is invalid Python,
which breaks type-stub generation.
Rename the argument to 'Tin' -- the name already used for Tensor inputs elsewhere
in the UniTensor binding -- and preserve the only previously usable legacy form,
put_block(**{'in': block}): a trailing (py::args, py::kwargs) overload detects an
'in' keyword, emits a FutureWarning, rewrites it to 'Tin', and forwards to the
typed overloads. Supplying both 'in' and 'Tin' raises TypeError, and argument
lists containing neither still raise TypeError so invalid calls are rejected. The
existing 'force'-deprecation warning messages are updated from 'in' to 'Tin'.
Add put_block_test.py covering the 'Tin' keyword, the deprecated 'in' keyword
(and its warning), the conflicting-argument rejection, and positional calls.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
725a8dd to
37e3210
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37e32102a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tools/generate_stubs.py reads the type annotations pybind11 bakes into the extension to produce the committed cytnx/cytnx/*.pyi stubs. Those annotations change between pybind11 releases (for example, 3.0.4 began emitting `typing.SupportsIndex` alongside `typing.SupportsInt` for integer arguments), so a floating `pybind11 >=3.0` requirement silently rewrites the committed stubs whenever the resolved version moves. Pin the build requirement to `pybind11 ==3.0.4` so the stubs generated by the following commit stay byte-for-byte reproducible. Bump this pin deliberately, together with a stub regeneration, when moving to a newer pybind11.
Ship committed type information with the cytnx wheel so IDEs and static type checkers can introspect the compiled pybind11 extension (cytnx.cytnx): autocompletion, hover docs, mypy/pyright/stubtest. - tools/generate_stubs.py runs pybind11-stubgen against a built extension and writes sanitized cytnx/cytnx/*.pyi files. They are committed and shipped unchanged in every wheel and conda package via the existing tool.scikit-build.wheel.packages = ["cytnx"] (the stubs live inside the package). The committed cytnx/cytnx/ directory has no __init__.py, so it does not shadow the extension at import time. - Generation is a deliberate offline step, never part of the build: pybind11-stubgen must import the freshly built, architecture-specific extension, which is impossible when cross-compiling and brittle in build sandboxes (e.g. the conda MKL build, whose prefix does not expose libatomic). A .pyi describes only the public API, so generating once and committing is sufficient for every platform. - Add the cytnx/py.typed marker (PEP 561), and the pybind11-stubgen generation dependency to the dev extra. - Raw C++ types that stubgen renders as a bare `...` (e.g. cytnx::Tensor in a docstring) are rewritten to typing.Any so the committed stubs are valid for every consumer. The stubs are generated with the lowest supported interpreter (Python 3.10) against the pinned pybind11 (==3.0.4, previous commit), so regeneration reproduces the committed files byte-for-byte. Co-Authored-By: Claude <noreply@anthropic.com>
37e3210 to
9e7e777
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e7e777dd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… Tensor The module-level Tensor factories (zeros, ones, arange, eye, identity, linspace, _from_numpy) are bound in generator_binding, which ran before tensor_binding registered the Tensor py::class_. With Tensor not yet registered when the factories were bound, pybind11 could not resolve their `-> Tensor` return annotation and emitted a raw C++ type, which pybind11-stubgen rendered as a bare `...` and tools/generate_stubs.py sanitized to `typing.Any`. Consumers of the shipped stubs therefore lost the return type: `cytnx.zeros([2, 2])` typed as Any, discarding all downstream Tensor method checking. Move generator_binding after tensor_binding so the Tensor class is registered first, and regenerate cytnx/cytnx/__init__.pyi: the factories now type as `-> Tensor`. Fixing it at the binding site keeps tools/generate_stubs.py free of a per-function return-type override. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c90a2e7c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@pcchen Can you help to review this? |
|
@pcchen @yingjerkao Is there any concern about this PR? |
yingjerkao
left a comment
yingjerkao
left a comment
There was a problem hiding this comment.
I will approve this, but my only concern is what the mechanism to sync with the C++ API changes ?
@yingjerkao Developers need to manually generate the stubs if the Python APIs change with C++ API. Per what was mentioned in this PR description, generating at build time is almost impossible. And generating before CI will lost flexibility to fine tune the stubs. We should add a section about the generating process in Contributing.md. I didn't add it because there was a PR creating the same file at the time. I will open another PR for adding the section. |
2 participants
Closes #585.
What this does
Ships committed type information with the cytnx wheel so IDEs and static type checkers can introspect the compiled pybind11 extension (
cytnx.cytnx) — autocompletion, hover docs,mypy/pyright/stubtest.tools/generate_stubs.pyrunspybind11-stubgenagainst a built extension and writes sanitizedcytnx/cytnx/*.pyifiles, which are committed and shipped — unchanged, platform-independent — in every wheel and conda package viatool.scikit-build.wheel.packages.libatomic), and forcespybind11-stubgen+ import-time deps into every build environment. A.pyidescribes only the public API, so generating it once and committing it is sufficient for all platforms; the committedcytnx/cytnx/directory has no__init__.py, so it doesn't shadow the extension at import time. Keeping the committed stubs honest with a CImypy.stubtestjob is planned (not yet wired into CI by this PR).cytnx/py.typedmarker....are rewritten totyping.Anyso the committed stubs are valid for every consumer.How other scientific packages ship stubs
Survey of how comparable packages with a compiled (C / Cython / C++) extension provide type information, along the axes this PR has to decide: do they ship a
py.typedmarker in their own wheel, are the extension stubs hand-written or tool-generated, are they committed or regenerated, do they have to import the built binary at build time to produce them, and how does CI keep them honest.py.typed.pyiin-treestubtest(imports only to verify)scipy-stubs.pyistubtest.pyifor C++ extsstubtest+ regenerate-and-diff.pyi(_imaging.pyi, …)mypyonly.pyiunderpyarrow-stubs/, copied into the wheel via CMakeinstall(... PATTERN "*.pyi")pandas-stubs.pyistubtest.pyistubtest.pyi.intemplates.pyiviapybind11-stubgen, sanitizedmypy.stubtestis planned (#585)Takeaways for cytnx's choices:
py.typed, not a separate*-stubspackage. Like NumPy, matplotlib, Pillow, PyArrow, and pydantic-core — and unlike SciPy/pandas, whose public-API stubs live in separately-installedscipy-stubs/pandas-stubs— a plainpip install cytnxis fully typed with nothing extra to install..pyiand regenerates them on every build via static codegen from YAML/templates (no binary import). cytnx can't mirror that:pybind11-stubgenreads the built extension by importing it, which is exactly what cross-compiled / sandboxed builds can't do. So cytnx generates once and commits the result, landing in the same place as the hand-written-and-committed crowd (NumPy, Pillow, …) while avoiding hand-maintenance.nofor every project, cytnx included — generation is a deliberate offline step, never part of the wheel build.mypy.stubtest; wiring an equivalent job into this repo's CI is left as follow-up work under Generate stub files #585.Sources: NumPy
multiarray.pyi+.spinstubtest task;scipy-stubs; matplotlib stubtest CI; Pillow_imaging.pyi; PyArrow developer docs;pandas-stubs; pydantic-core.pyi; PyTorchtorch/_C/__init__.pyi.in+.gitignore.Binding fixes required for valid stubs
Pre-existing bindings produced signatures that are invalid or lossy Python and broke the generated stubs:
Network.constructboundoutrkwith no default although the C++ API defaults it to0(a non-default-after-default signature). Now bound with its= 0default.UniTensor.put_block/put_block_named their leadingTensorargumentin, a Python reserved word. Renamed toTin(the convention used elsewhere in the UniTensor binding). Positional calls are unchanged; the only form that could pass it by name —put_block(**{'in': block})— keeps working via a trailing(*args, **kwargs)overload that emits aFutureWarningsteering callers toTin, while conflicting/invalid argument lists raiseTypeError.Tensorfactories (zeros/ones/arange/eye/identity/linspace/_from_numpy) typed as-> typing.Anybecausegenerator_bindingran beforetensor_bindingregistered theTensorclass, so pybind11 could not resolve their-> Tensorreturn. Reorderinggenerator_bindingaftertensor_bindingmakes them type as-> Tensordirectly (binding-layer fix, no stub post-processing).Notes
masterafter Remove untested tn_algo Python bindings for non-functional C++ code #922 removed thetn_algoPython bindings, so the committed stubs no longer includetn_algo.pyi.pybind11is pinned to==3.0.4in[build-system].requires. pybind11 controls the type annotations baked into the extension thattools/generate_stubs.pyreads, so a floating version silently rewrites the committed stubs (3.0.4, for example, annotates integer arguments astyping.SupportsInt | typing.SupportsIndex); pinning keeps them reproducible, and the pin is bumped deliberately alongside a regeneration. The pin lands before the stub-generation commit, so the stubs are produced against it directly with no follow-up regeneration commit.__version__tracksversion.cmake.Verification
pytest pytests/put_block_test.py: 12 passed (positional,Tin=, deprecated**{'in': ...}withFutureWarning, and conflicting-argument rejection).cytnx.zeros→Tensor,cytnx.linalg.Svd,UniTensor.put_block).Follow-ups (sub-issues of #585)
*_conti.pymodules have pre-existing type-checker errors surfaced bypy.typed.help()show only signatures;pybind11-stubgendoes not synthesize docs.Tensorfactories (by orderinggenerator_bindingaftertensor_binding), and Bind ExpH/ExpM with per-dtype overloads + Python-layer fixes (toward clean stubtest) #915 (stacked on this PR) cleans uplinalg.ExpH/ExpM. The remaining items — per-dtype operator overloads (~5600overload-cannot-matchin__init__.pyi, including the__eq__incompatible-override pattern), the__hash__/__eq__markers, the over-broadput_blockfallback, and the raw-C++-typetyping.Anyrewrites — plus removing the generator's post-processing and enabling amypy.stubtestCI gate, are tracked in Fix stub-quality issues in the pybind layer instead of post-processing in generate_stubs.py #928.🤖 Generated with Claude Code