-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Report that NamedTuple
and dataclass
are incompatile instead of crashing.
#18633
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
Report that NamedTuple
and dataclass
are incompatile instead of crashing.
#18633
Conversation
…rashing. The fix is pretty simple. I could not find a situation where combining `NamedTuple` and `dataclass` makes sense, so emitting an error seems sensible.
for more information, see https://pre-commit.ci
This comment has been minimized.
This comment has been minimized.
@@ -965,6 +965,9 @@ def dataclass_tag_callback(ctx: ClassDefContext) -> None: | |||
|
|||
def dataclass_class_maker_callback(ctx: ClassDefContext) -> bool: | |||
"""Hooks into the class typechecking process to add support for dataclasses.""" | |||
if any(i.is_named_tuple for i in ctx.cls.info.mro): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we are at it: what about TypedDict
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dataclass
is also not really compatible with TypedDict
, but here, things work a little differently, and Mypy's understanding seems to be consistent with Python's runtime behaviour:
from dataclasses import dataclass
from typing import TypedDict
@dataclass
class A(TypedDict):
i: int
a = A(i=1)
assert a["i"] == 1
a.i
# Mypy -> error: "A" has not attribute "i" [attr-defined]
# Python -> AttributeError: 'dict' object has no attribute 'i'
First I was excited when I saw that issue: "wow, we can make a dataclass with convenient unpacking, nice!". However, it really doesn't work at runtime: from dataclasses import dataclass
from typing import NamedTuple
@dataclass
class Foo(NamedTuple):
x: int
y: str
Foo(0, '') results in
So this is probably the most reasonable fix. Can we expand the message or add some note about runtime incompatibility? It really isn't immediately obvious why not use that combination. |
Maybe "A NamedTuple cannot be a dataclass ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Co-authored-by: sobolevn <mail@sobolevn.me>
Co-authored-by: sobolevn <mail@sobolevn.me>
Co-authored-by: sobolevn <mail@sobolevn.me>
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
…rashing. (python#18633) Fixes python#18527 The fix is pretty simple. I could not find a situation where combining `NamedTuple` and `dataclass` makes sense, so emitting an error and just not applying the dataclass transformations seems sensible. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: sobolevn <mail@sobolevn.me>
Fixes #18527
The fix is pretty simple. I could not find a situation where combining
NamedTuple
anddataclass
makes sense, so emitting an error and just not applying the dataclass transformations seems sensible.