-
Notifications
You must be signed in to change notification settings - Fork 64
dataclasses integration / defer __match_args_required__? #79
Comments
I'd want to know of a solid use case for that extra flag -- IOW KISS. When would you want to have a positional argument to To show how it works using both dataclasses and regular classes you could leave BinaryOp a regular class but turn UnaryOp and VarExpr into dataclasses. BinaryOp has a precedence attribute that's not a parameter, so you'd have to define a |
Well, it's a two-way door in any case, we can always add it later if someone decides they need it. I'm fine with leaving things as-is. |
Do we have any plans for |
I propose to leave that unset. Users wishing that feature can set it. |
The PEP says we set it. So either we should set it, or change the PEP! |
So set it if not already set. To 0, I suppose. What difference does make though? |
|
I privately worry that will constrain us. While 'Point(0)' seems perhaps an odd shortcut for 'Point(0, _)', I am not so sure that we want to disallow 'Point(x=0)'. |
Well, we could just strike the line from the PEP. Like you said, anyone who wants to require args can set I could go either way, honestly. The deconstruction signature will never be perfectly compatible with the constructor signature, but it does seem that we haven't found a real-world use-case where we're willing to set So either we should choose to set it for named tuples and dataclasses, find a more compelling example for the PEP, or consider dropping it from the spec. |
What do others think? @Tobias-Kohn ? |
I'm slightly in favor of killing it off. Looking it up and validating it takes time and effort, and we haven't really found any good use cases. Since we probably wouldn't add it to any built-in types, we could always add it back later with minimal disruption. |
Plus, I think it currently holds the world record for "most ridiculously long dunder". |
For some background, here's the comment that first suggested it: Later, @ilevkivskyi said:
|
So you propose to kill it completely? That sounds fine to me, we can add it to Rejected (or Postponed) ideas. |
I am also at most lukewarm towards the |
I've added code to Brandt's reference implementation to support dataclasses as specified in the PEP. This causes the
@dataclass
decorator to generate a__match_args__
property for the target class.However, my implementation is minimal, and leaves out a number of things that could have been done.
The various properties that
@dataclass
creates are controlled by two flags: one flag that is global to the class, and one that is per-field. So for example, there is aninit
flag that controls whether an__init__
method should be generated; there is another per-fieldinit
flag which determines whether a given field should be included in the constructor signature.My implementation does not include either of these flags for
__match_args__
. It always generates a__match_args__
attribute, unless the class already has such an attribute defined; I see no reason not to generate it.As to which fields are included in
__match_args__
, I simply include all of the fields that have theinit
flag set to True - in other words, if it's a constructor parameter, I also include it in__match_args__
. Fields which are not constructor parameters must be matched by name.However, I could envision a more flexible approach, in which there is a per-field
match
flag, which would default to the per-fieldinit
flag (which in turn defaults to True). Thus, if you didn't specify this flag, it would default to including all of the constructor params as match args, but you could also override this on a per-field basis.(I'm also tempted to update my example to leverage dataclasses, although I would want to show how match works both with and without dataclasses.)
The text was updated successfully, but these errors were encountered: