-
-
Notifications
You must be signed in to change notification settings - Fork 385
Post-Init-Only arguments #342
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
Comments
But ah, if you declare it like that static tools will think it's a field by default, no? My gut feeling is additional args to init don't really deserve to be in the class body like that. |
Fuck yeah, you’re totally right. It should be an argument to |
i would like to point out that "named constructors" are commonly used for such setups allowing more clear spelling @attr.s
class C:
i = attr.ib()
j: int = attr.ib()
@classmethod
def from_database(cls, i, database):
j = database.lookup('j')
return cls(i, j) so depending on the actual needs and intended use-cases it may be entirely sensible to just leave it to the syntax |
Yeah, I love named classmethods as named constructors. That’s one of the reasons why I’m not all in on the idea. But I’d like to watch how it’s embraced. It doesn’t help to die alone on a hill even if you’re right. |
I think this would be amazing and it's one of the cool things about dataclasses that attr's doesn't have |
I just came across this discussion while at a loss trying to figure out a right way to use attrs in a non-attrs hierarchy. I think that the following simple addition would strictly extend the domain of application of attrs without impacting anyone who does not need it. A new option (say, extra_args) to @attr.s(extra_args=True)
class Child(NonAttrsClass):
a = attr.ib()
b = attr.ib()
def __attrs_post_init__(self, base_attr):
super().__init__(self, base_attr) would cause the following def __init__(self, a, b, *args, **kwargs):
...deal with a and b...
self.__attrs_post_init__(*args, **kwargs) This would already cover a great deal of situations where attrs falls short of providing a satisfactory solution. One could even argue that the option is superfluous: instead, |
This is solvable with a custom @attr.s(extra_args=True)
class Child(NonAttrsClass):
a = attr.ib()
b = attr.ib()
def __init__(self, a, b, c):
self. __attrs_init__(self, a, b)
# ... do something with c ... Which also has the advantage of not having an opinion in |
Thanks for the prompt reply, this is bringing me much further already. Much appreciated! This solution has one drawback however: it makes me repeat myself. Each attribute is now listed three times (declaration, Also, I don't see the above proposal as an opinionated solution to the specific problem of non-attrs class hierarchies (in particular, it does not allow to invoke the base class constructor before attrs-generated |
Yeah… I'm just wonder out loud about how many variations of how one could change |
Back to @pbourguignon's proposal, would it be feasible to just notice extra arguments in the signature for @attr.s()
class Child(NonAttrsClass):
a: int
b: str
def __attrs_post_init__(self, x: bytes, y: bool = False):
super().__init__(self, x, y=y) Which could be use used thusly: child = Child(1, "foo", b"")
assert child.a == 1
assert child.b == "foo"
assert child.x == b""
assert not child.y |
I think there is a possibility to keep it stupid simple by generally changing the attrs-generated def __init__(self, attr1, attr2, *args, **kwargs):
...do what it already does...
if hasattr(self, __attrs_post_init__):
self.__attrs_post_init__(*args, **kwargs) # This raises already on invalid/missing arguments
elif len(args) + len(kwargs) > 0:
raise ArgumentError("Extra arguments passed to __init__, but not used.") This seems generic enough so I wouldn't expect further variation to be called for. It still does not cover the case where a base class needs to be initialized before attrs-generated On the other hand, the solution you suggest has the added benefit of |
As for your question:
I'd say inspect has all you need for that. |
I think that's pretty important for typing. |
Yes, it is, and definitely calls for the more elaborate solution you outlined. |
@pbourguignon just wondering, did you make any progress on this? I would definitely use this feature if it existed! The |
@sinback Sorry, no, not yet. I settled on a custom init solution for my current needs. |
@hynek - two years after the "on hold" status - have your thoughts changed about this feature? I was recently looking for it. |
I'm squarely in the "named constructor" / "classmethod constructor". I find the amount of complexity this would introduce (especially taking into account subclassing) just isn't worth it. |
For those interested, I had the same problem where The decorator saves writing a custom This might not the best solution for my particular use-case, but it might be useful to others. |
I agree that this approach seems like a clean solution in many cases. The problem I have with this approach is that most of my arguments need to be passed from the classmethod constructor to the default |
I don't disagree, I just haven't seen a proposal where the outcome would justify the induced complexity. :-/ |
I'd like to note that even with the handwritten approach init only arguments have horrendous maintenance costs over time In pytest the overly tricky collection node constructors are a major showstopper for direly needed refactorings we want to land since well before 2016 |
I'm not entirely sure how attrs could even best support inheritance -- it seems dataclasses just kinda throws its hands up and hopes for the best >>> from dataclasses import dataclass, InitVar
>>> @dataclass
... class C:
... x: InitVar[int]
... def __post_init__(self, x):
... print(x)
...
>>> @dataclass
... class D(C):
... x: int
...
>>> D(42)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<string>", line 4, in __init__
TypeError: C.__post_init__() missing 1 required positional argument: 'x' |
So data classes have the concept of init-only variables:
Thanks to converters I think the need in attrs is much lower, but we should keep an eye on the popularity of it.
I think I could live with something like:
I don’t think it should be shoehorned into
attr.ib()
.To be clear: the example above is totally possible with attrs, it's just a bit more clunky.
The text was updated successfully, but these errors were encountered: