-
Notifications
You must be signed in to change notification settings - Fork 76
with_args, kw_only, slots support in >=3.10 #205
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
Conversation
marshmallow_dataclass/__init__.py
Outdated
def dataclass( | ||
_cls: Type[_U] = None, | ||
*, | ||
repr: bool = True, |
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.
Do we lose much if we replace all the keyword args that we just pass off to dataclasses.dataclass
with **kwargs: Any
(in the implementation, not in the overloads)?
Then the overloads could be conditional (on python version), but we'd only need one implementation, thus avoiding duplication of the code in the function body.
marshmallow_dataclass/__init__.py
Outdated
""" | ||
This decorator does the same as dataclasses.dataclass, but also applies :func:`add_schema`. | ||
It adds a `.Schema` attribute to the class object | ||
# _cls should never be specified by keyword, so start it with an |
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.
Apparently, mypy recognizes parameters whose names start with a double-underscore as positional-only.
(This seems only to be documented in the mypy cheatsheet. But the typeshed stub for dataclasses.dataclass
uses the syntax.)
Anyway... we should probably change _cls
to __cls
.
marshmallow_dataclass/__init__.py
Outdated
def dataclass( | ||
_cls: Type[_U], | ||
*, | ||
repr: bool = True, |
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 were messing around here, we should fix the typing of our dataclass
to match typeshed’s typing for dataclasses.dataclass
That allows for
@dataclass
class A:
...
@dataclass()
class B:
...
and
@dataclass(**kwargs)
class C:
...
But does not allow for providing the class and keyword params all in one go. I.e. the following is not supported by dataclasses.dataclass
:
class D:
...
D = dataclass(D, frozen=True)
marshmallow_dataclass/__init__.py
Outdated
# underscore. The presence of _cls is used to detect if this | ||
# decorator is being called with parameters or not. | ||
def dataclass( | ||
_cls: Type[_U] = None, |
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.
Implicit optional parameter types are deprecated1. Should fix to __cls: Optional[Type[_U]] = None
while we're in here. (Though there are admittedly a lot of other places in our code where we rely on implicit optional...)
Footnotes
Hello, I'm looking to use |
I made a new PR, very similar to this. Hoping to get this revived 🤞 @antoine-chopin |
23855f9
to
9812e50
Compare
Add (new in 3.10) arguments to
@dataclass
Similar to #204 , with sys.version_info guarding.
I tried using a common dispatch handler, but I think custom mypy handling of this class was breaking dispatch, and the forward reference tests were failing. Open to suggestions.