experimental-inspect: mark disjoint base classes with typing_extensions.disjoint_base - #6362
Conversation
| // Being a disjoint base depends on the instance layout, so the decorator list is | ||
| // picked by the compiler. | ||
| let disjoint_base = IntrospectionNode::List(vec![PyExpr::module_attr( | ||
| "typing_extensions", |
There was a problem hiding this comment.
Genuine question: are stubs going to be considered valid by all tools even if typing_extensions if not installed?
There was a problem hiding this comment.
| checker | verdict on @disjoint_base |
|---|---|
| mypy 1.0.1 – 1.17.1 | error: Module "typing_extensions" has no attribute "disjoint_base" [attr-defined] |
| mypy 1.18.1 (released 2025-09-11) and up (through 2.3.1) | clean |
| pyright ≤ 1.1.405 | error: "disjoint_base" is unknown import symbol |
| pyright 1.1.406 (released 2025-10-02) | clean |
=> Seems to be actually typechecker dependent, since they ship their own typeshed copies AFAIK.
So this PR would lead at least to a minimum typechecker version dependency, but the versions required are ~10-ish months old. Dunno whether this is okay or not.
There was a problem hiding this comment.
What about other type checkers (ty, pyrefly etc?)
I think it'd be better IMO emit typing.disjoint_base here from the macro rather than assume they all support typing_extensions unconditionally
We can then have some options in pyo3-introspection:
- we could replace
typingwithtyping_extensionsfor this import - we could have some kind of version-based import
- we could even just drop this decorator if targeting codebases older than 3.15
We could potentially give users some kind of control over which happens with config.
There was a problem hiding this comment.
+1
Dropping the decorator looks like a good safe first step to me
There was a problem hiding this comment.
I'm strongly suggesting not to do this (as it actually wouldn't be spec-compliant), but keep typing_extensions.disjoint_base as-is. Specifically not replace it with typing (heavily reduced Python version compat and not spec-compliant), not a version-based import (not required, and also not done in typeshed, and wouldn't increase compatibility), and not drop the decorator for code bases older than 3.15 (same; i.e. not required, but actually the type checker version instead is the version gate).
But a config flag where one could turn the feature off all together sounds reasonable, if we really want to support almost ~1 year old type checkers in an upcoming PyO3 release (do we?).
Explanation of this is here.
To give full signal on minimum version requirements:
- All of this hinges on the first release of
typeshedwheredisjoint_basewas introduced, as this is what actually resolvestyping_extensions.disjoint_basein downstream typechekers. PR and release oftyping_extensions4.15.0 was 2025-08-24/25 respectively mypyintroduced support in 1.18.1, released 2025-09-11tyintroduced support in this PR, 0.0.1-alpha.20 release was 2025-09-3pyreflyintroduced support in 0.29.0, released 2025-10-27pyrightintroduced support in 1.1.406, released 2025-10-01
There was a problem hiding this comment.
Thank you for the investigation! I am not strongly against using typing_extensions.disjoint_base if it's supported by the latest versions of the type checker but I am still a bit scared of breaking old mypy. But 1.81.1 still support python 3.9, the earliest version of pyo3 we also support so it looks fine to me.
converting from typing_extensions.disjoint_base to typing.disjoint_base can be done via conditional compilation if the build target 3.15+. Glad to postpone that to a follow-up.
Would love to hear @davidhewitt opinions though.
| // Being a disjoint base depends on the instance layout, so the decorator list is | ||
| // picked by the compiler. | ||
| let disjoint_base = IntrospectionNode::List(vec![PyExpr::module_attr( | ||
| "typing_extensions", |
There was a problem hiding this comment.
What about other type checkers (ty, pyrefly etc?)
I think it'd be better IMO emit typing.disjoint_base here from the macro rather than assume they all support typing_extensions unconditionally
We can then have some options in pyo3-introspection:
- we could replace
typingwithtyping_extensionsfor this import - we could have some kind of version-based import
- we could even just drop this decorator if targeting codebases older than 3.15
We could potentially give users some kind of control over which happens with config.
|
There were a few suggestions spread across different places about TL;DR # mod_te.pyi`
from typing_extensions import disjoint_base
@disjoint_base
class Foo: ...# use_te.py
from mod_te import Foo=> Even if targetting Python version 3.10 in the type checker, this checks just fine. If I use Some facts
Every Python version >= 3.0 shall support everything in And this explicit rule about features not present in every Python version having to be imported from
Why is that? Every type checker ships its own copy of |
|
I asked at python/typing#1532 which seems to propose that I would therefore like to go for the following proposal:
I think for this PR it would be good enough to just land the |
|
@davidhewitt Thank you! I tend to slightly prefer the approach in #6384 that pushes the choice between An other "middle" ground option is to set the minimum supported python version in the introspection data and make pyo3-introspection rely on it. WDYT? I can live with any of the three options. |
Sadly I don't think this is true; often PyO3 is built with a version-specific API which happens to be the current interpreter rather than a minimum version. This statement is only true if an |
Good point. But in this case, aren't all type checkers resolving imports using the current interpreter too? So, the version used in pyo3 will still be the correct one? Stubs are build/wheel-specific, the "minimum python version" should be understood as the one of the build/wheel not the one of the crate/package |
|
I guess we haven't fully resolved exactly when we expect packages to generate their stubs. I am unsure whether best practice is going to be to run stub generation as a static step and commit stubs into the repository, or to generate stubs as part of wheel build. In the latter case - you're definitely right that current interpreter will match what the stubs contain. If stubs are checked in - I guess as long as users are aware that they should generate stubs with their minimum supported interpreter, it'll be fine. For the sake of unblocking these, let's go with the approach in #6384 and document that stub generation is sensitive to the interpreter version, so checked-in stubs should use the minimum-supported interpreter. |
|
@davidhewitt & @Tpt thanks so much for the investigation and discussion! For now I switched to the #6384 approach: the macro now emits To give a datapoint: To make docs rendering and typechecking statically a bit easier, in our packages we tend to check generated stubs in, instead of having to always dynamically generate them for mentioned CI jobs. Usually we do ABI compliant Python 3.12+ builds, and then have a CI job that checks whether the a generated typestub matches byte-to-byte with the one already checked in. |
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks, let's merge this to move forward 👍
|
Great to see this moving forward. Thank you!
Sadly this is not fully true because stubs might still be partial: if an exposed function is gated by The amazing thing would be to generate stubs with proper version and platform gates but it's sadly not doable with the "introspection data is generated during macro expansion" idea (if you have |
What was wrong
#[pyclass]stubs never emitted PEP 800@disjoint_base, so stubtest reported 6 errors for the 6 non-final pyclasses in pytests.The obvious rule does not hold
"Not final implies disjoint base" is wrong. stubtest's check (mypy
_is_disjoint_base) is purely__basicsize__/__itemsize__differing from__base__. Measured on a scratch extension:#[pyclass(subclass)] struct Base {}#[pyclass(subclass, frozen)] struct FrozenEmpty {}#[pyclass(extends = Base, subclass)] struct MidEmpty {}#[pyclass(extends = PyDict, subclass)] struct DictSub {}#[pyclass(extends = PyDict, subclass, frozen)] struct DictSubFrozen {}A blanket rule would decorate three of these wrongly, producing the inverse stubtest error and telling type checkers that legal multiple inheritance is impossible.
The fix
The exact criterion is whether the class adds to its base's instance layout, which is
size_of::<PyClassObjectContents<T>>() > 0. That is the same quantity PyO3 already feeds intotp_basicsize/Py_tp_extra_basicsize(seesrc/pycell/impl_.rs,BASIC_SIZE).