-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom dataclasses are not recognized as dataclasses #15843
Comments
Yes, following #14849, mypy now infers a much more precise signature for @ikonst, would it be possible to just fallback to the signature typeshed gives if mypy sees an instance of a class that hasn't been constructed with (It's not a huge issue for typeshed, though, we can just comment out the test for now and that's not the end of the world.) |
(Although, It would be cool to have the typed replace function for custom dataclasses 😄) |
Well sure, but I'm guessing that would be significantly more complex :) |
Yup, fair enough. Thanks for taking a look at this so quickly. |
BTW, we also have a false positive problem, assuming every import typing
_T = typing.TypeVar("_T")
@typing.dataclass_transform()
def create_model(cls: type[_T]) -> type[_T]:
cls.__init__ = lambda *args, **kwargs: None # type: ignore
return cls
@create_model
class C:
a: int
c = C(a=42)
typing.reveal_type(c.__dataclass_fields__) Runtime:
mypy:
|
To eliminate the "Argument 1 to "replace" has incompatible type "X"; expected a dataclass" error in the issue's code, we can make a simple change: -if self._spec is _TRANSFORM_SPEC_FOR_DATACLASSES:
- self._add_internal_replace_method(attributes)
+self._add_internal_replace_method(attributes) If we want to go ahead with it, see #15915. This would mean that you can call Right now |
Would it be worth exploring saying (in a PEP I guess) that it should be guaranteed that:
Are there many other examples of custom dataclasses that don't delegate construction to the dataclass decorator? Could the Pydantic custom dataclass be easily made to conform to this idea? If not by delegating to This whole mess would have been avoided if dataclasses had been a base class from the start. Instead they're implemented using a decorator that acts like a base class (provides functionality and makes interface promises). I see custom dataclasses as virtual subclasses of this dataclass ABC, so I think it might make sense to have these virtual subclasses make the same promises. |
Pydantic predates dataclasses so whatever approach they took - decorators, base classes, meta classes - is not a consequence of any dataclass design decisions, whether good or bad.
|
Okay. My main point was that I see the dataclasses decorator as analogous to a base class. And I see custom dataclass decorators as analogous to subclasses of that base class. So in my personal view, And that's really my question. Can we make it always true? Are there many examples of custom dataclasses that aren't compatible with dataclasses? And if so, how easily can they be made compatible? To make them compatible, I propsed using dispatch, but registration of a replace/field method might be another alternative. |
If I understand correctly, the Data Class Transforms mechanism was introduced not to create "custom data classes" (a runtime concept) but to let 3rd party libs tell mypy & pyright how to understand a custom type without having to write a plugin (a typing concept). That's why it's It's possible that PEP-681 was misunderstood and/or its use evolves differently than planned. Using Sourcegraph, I found them split into multiple categories:
By "wraps dataclasses", I mean that they make a re pydantic, unlike what's been said in the thread, I didn't see it produce a import dataclasses
import pydantic
from typing import reveal_type
class User(pydantic.BaseModel):
name: str
age: int
user = User(name='John', age=42)
print(f'{dataclasses.is_dataclass(user)=}')
reveal_type(user.__dataclass_fields__) results in
My summary
|
Just FYI: The flax dataclass also wraps
Right. The question is what are we telling mypy and pyright? Are we promising only fields or the dataclass interface ( What is the best way to make custom dataclasses support the dataclass interface? If a custom base class like this were added (see here), and if it were made to support inheritance, then we could do: class MyCustomDataclass(MyDataclassInstance):
def __init_subclass__(cls):
super().__init_subclass__()
# do your custom behaviour here
class SomeDataclass(CustomDataclass):
x: int
y: float
reveal_type(SomeDataclass.__dataclass_fields__) # dict[str, Field[Any]] If you're going to provide type checking for |
You're right, flax has two cases - kw_only_dataclasses.dataclass that clearly produces a dataclass, and ModuleBase is a hack for something that'd always be a base class of a dataclass in runtime.
Right now, it promises that, quoting PEP-681, "certain decorator functions, classes, and metaclasses provide behaviors similar to dataclasses".
I'm not sure I'd start off with "custom dataclasses" since there's scant evidence anyone is doing it. For example, pydantic is very close to what I'd call a "custom dataclass" and it never bothered emulating dataclasses for runtime purposes. Focusing on "Wraps dataclasses" makes for a stronger case.
The path of least resistance is to simply add a parameter to Then the question of whether |
That would perfectly satisfy me. For motivation, all of the Jax-ecosystem dataclasses would benefit (flax, chex, my tjax library). It would allow type checking of
By custom dataclasses, I just mean users of the |
Really well-written! Thanks for taking the time to post that. |
We [already synthesize](#15843 (comment)) `__dataclass_fields__` for all classes including `@dataclass_transform`'d ones, thus assume more than PEP-681 says. We might as well assume `dataclasses.replace` applies on all same classes. This way we risk false positive since it'll raise in runtime. Fixes #15843.
I'm looking at this again, and there's something I don't understand: the function that I decorated with dataclass-transform (here called |
Mypy heavily special-cases dataclasses via a custom plugin that means it can have a far more precise understanding of dataclasses than could be achieved if it just looked at the stubs alone. (I believe all other major static type checkers take a similar approach.) Whether or not a type is annotated as |
Sorry, I do understand that. I'm just confused about this issue in particular. MyPy has figured out that Essentially, it appears that MyPy is ignoring the return type annotation of dataclass-creators (call it |
Can you open a new issue with a minimal repro of what you're trying to do, that doesn't currently work when you're using mypy's master branch? |
@AlexWaygood Done: #16241 |
Feel free to skip to the bottom of the code. The reset is custom dataclass specification.
It appears that MyPy recognizes that something is a dataclass through some means other than the protocol. See here for background: python/cpython#102699
The text was updated successfully, but these errors were encountered: