pylance-release
pylance-release copied to clipboard
`dataclass_transform` incorrectly infer `__init__` kwargs
Environment data
- Language Server version:
Pylance language server 2022.9.10 (pyright aba41864) starting
- Python version (& distribution if applicable, e.g. Anaconda): 3.8
Code Snippet
Reproduction instruction:
import typing_extensions
@typing_extensions.dataclass_transform(kw_only_default=True)
class A:
x: int
class B(A):
y: int
B(y=1) # pylance expect B(x=, y=), but should instead be `B(y=)`
Explaination: In the above snippet, B
is a dataclass-like object (because it inherit from a dataclass_transform
class). However A
itself should NOT be a dataclass-like object.
Semantically, dataclass_transform
should be similar to:
import dataclasses
class A:
x: int
def __init_subclass__(cls):
dataclasses.dataclass(cls)
class B(A):
y: int
B(y=1) # A is not a `dataclass`, so `B.__init__` is `(y=)` NOT `(x=, y=)`
Context: https://github.com/google/flax/issues/2416#issuecomment-1239658713
Our dataclass_transform
support comes from Pyright, the type checker that Pylance is based on. This is a duplicate of https://github.com/microsoft/pyright/issues/3907, but we appreciate your thoughts on this issue.
I don't think this is a duplicate of https://github.com/microsoft/pyright/issues/3907. The other bug is about whether __init__
method synthesis is skipped based on the presence of an explicit __init__
within the parent class.
In this issue, the __init__
method synthesis is assumed, but there's a question of whether it should include fields from a class that is decorated with @dataclass_transform
directly.
When we first authored PEP 681, we said that @dataclass_transform
was intended to be used in two contexts:
- To decorate a function that, itself, is designed to work as a class decorator and modifies the decorated class to give it dataclass-like properties. The stdlib
dataclass
is implemented as such a function. - To decorate a class that is intended to be used as a metaclass that implements dataclass-like functionality for any class that is created by that metaclass. This is how pydantic's
ModelBase
class works.
We later added a third case, which is used in the code sample above:
- To decorate a base class that implements dataclass-like functionality for any class that derives from it.
This third case was added late in the review process, and I can see now that there were aspects that we didn't consider when spec'ing it or implementing it. We weren't clear in the spec about whether fields within the decorated base class would be treated as dataclass fields. Pyright's current implementation assumes that they are, which is why x
and y
are both assumed to be present in the synthesized __init__
method. @Conchylicultor, I can understand why you assumed that the fields within the base class should not be be included here. It's a reasonable assumption.
I think this is something we may need to go back and clarify in the PEP. Once clarified, we can make sure that all type checkers (including pyright) conform to the clarified spec.
My thinking was that the core issue in both scenarios was where we believe the boundary is between library code (the implementation of the dataclass-like behavior) and user code (the consumer of that behavior).
I am not entirely sure I'd expect B(y=)
in place of B(x=, y=)
, if anything B(x=, y=)
makes for sense in my current mental model.
That said, I am wondering if dataclass_transform
admits programmatically modifying fields? In Flax, inside __init_subclass__
we programmatically add a couple of attributes by modifying .__annotation__
, this is currently not detected by pylance
. Is this one of:
- Not supported by PEP 681
- Supported but a current bug in pylance
- We are missing something to make pylance happy
@cgarciae, it's option 1 -- not supported by PEP 681. There's currently no way to tell dataclass_transform
that you'll be adding arbitrary extra attributes to the affected class.
@Conchylicultor, reading this again it's now clear to me that this is different from https://github.com/microsoft/pyright/issues/3907, and also that the expected behavior you described above is clearly correct -- attributes on the class decorated with dataclass_transform
(or that class's base/ancestor classes) should not be considered to be fields.
I will clarify this in PEP 681 and we'll get the behavior fixed in Pyright/Pylance as well.
This has been fixed in Pyright. The fix will be available in the next prerelease build of Pylance.
This issue has been fixed in prerelease version 2022.10.21, which we've just released. You can find the changelog here: CHANGELOG.md
Awesome! Thank you!