-
- Notifications
You must be signed in to change notification settings - Fork 408
Description
First of all, thank you for building such a wonderful package! It is now an essential part of both my work and my hobby projects.
This issue is similar to #481, but I think it's more suitable to open a new issue for discussion instead of reviving a 4-year-old thread.
kw_only can be set both on the class decorator and on attributes. When kw_only=False on the class (this is the default), one can set kw_only=True on attributes to make only those attributes keyword-only in the constructor. However, when kw_only=True on the class, all attributes (including those defined in base classes) become keyword-only, and setting kw_only=False on attributes will not work.
The following code snippet implements the logic described above:
Lines 564 to 566 in 7091b1f
| if kw_only: | |
| own_attrs = [a.evolve(kw_only=True) for a in own_attrs] | |
| base_attrs = [a.evolve(kw_only=True) for a in base_attrs] |
In my opinion, this is a little confusing, since many of the other options that exist on both the class and attributes do give you a way to opt-out for a subset of attributes. For example, eq (and friends), repr, and init all allow you to exclude an attribute from the auto-generated functions. It feels natural to assume the same for kw_only; I don't have exact stats, but I know quite a few coworkers who were surprised by the current behavior.
It would also be useful to allow overriding. For example, I have a class whose ideal constructor signature should look like this:
SomeObject(x, y, *, kw1=..., kw2=..., kw3=..., ...) # imagine there's a lot more kw-only argswhere x and y are required, and all the rest are optional with defaults, usually some obscure setting that can be tweaked but are rarely tweaked. To implement such an interface, I would have to set kw_only=False on the class, and kw_only=True on most of the attributes. To make matters worse, I need to do the same for all new attributes in all of its subclasses as well. It would be beneficial if we could just set kw_only=True on the class, and override a few attributes with kw_only=False.
If we are to allow overriding kw_only on attributes, this is how I imagine it should behave:
- The class's
kw_onlydefaults to False, as it does today. - The attribute's
kw_onlydefaults to None. If it is set to True or False, then it stays kw-only or non-kw-only regardless of the class's setting; if it is not set (None), then it reflects the class'skw_onlyvalue. - Note that this also means that a subclass cannot control the
kw_only-ness of its base attributes.
I'm open for debate on the semantics and I'd love input on how this can be improved or made more natural or useful.
Now, I would love to put up a pull request for this change, but I realize that this would break backwards-compability, so I'd like to seek permission from maintainers before I go ahead and do this. I'm also open to introducing a new class-level option (e.g. kw_only_overridable, or something less horrendous) that's disabled by default, similar to how collect_by_mro works. Please let me know if you think this proposal makes sense, and how we should move forward. Thanks!