-
-
Notifications
You must be signed in to change notification settings - Fork 373
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
Suggestion: consider changing slots
default from True
to False
#972
Comments
Firstly, changing such a profound default is just not possible, because that would be massive breach of backwards-compatibility. We took a looong of time to agree on them, but given how widely I'm not sure what you mean by changing the semantics of OOP1 or how inheritance is "often broken by slots". AFAICT it only triggers a few edge-cases, otherwise our bug tracker would look very differently. The vast majority of users – in my estimation of course – do benefit from slots being on by default. Them being faster is just one nice side effect, I (and many users & contributors) consider the strictness slots introduce, that you pointed out, a huge upside. Accidentally setting the wrong attribute due to a typo and wondering why the actual attribute has an unexpected value is a very common bug in dynamic languages. Helping our users to write more correct software is one of our priorities. Being able to monkeypatch classes is not. I will grant you that the "no surprises" philosophy might be impacted by this default, and I'm taking this issue as an inspiration to make the consequences of slots more prominent in the documentation, however the API impact of slots is zero and I don't consider slotted classes to be non-regular. Slotted classes have been around forever and there's a reason dataclasses I'm genuinely sorry your trade-off calculation is different, but there's nothing I can do except recommending Footnotes
|
Thanks so much for the thoughtful and friendly reply :). Understood if this is a non-starter... the point about backwards-compatibility is granted, and I agree there is value in mitigating a common class of error (instance variable typos). I'll admit that though I've been writing Python code since, like, 2005, I wasn't really familiar with slots as a language feature until now, so maybe I'm just naive, but below is a simplified repro of what tripped me up as a new attrs user. In my real use case there was inheritance involved, which I thought was related, but now that I've simplified it so much, I see it has nothing to do with inheritance. I do still feel it's fair to say that a commonly-used aspect of the semantics of OOP (by which I just mean writing and using classes) has changed -- namely, the expectation that instances can "mask" their references to class variables, which gives rise to a commonly used idiom. I have the impression that it's common practice to define classes with class variables that represent (default/initial values of) instance state, with the expectation that instances will, in their normal course of business, assign to an instance variable with the same name in Anyway, slots break it, which is cool, it's just I wouldn't have called it a weird/rare edge case out there in the world of writing ordinary python classes, at least as I've seen it practiced. That said, I could be wrong about common practice, and your earlier points probably preclude changing the default. You might consider being more up-front about the slots default in an early section of the documentation (of course just a little note/footnote with a link to more info). You could even highlight the impact on this idiom, if you agree that it's a common one. Thanks again! from attrs import define
@define(slots=True)
class MyClass:
"""
Just an example.
"""
# ... maybe there are a bunch of fields here...
# then, a "normal" class variable for some other purpose. It seems like I
# should be able to have a class variable that isn't an attr.field here and
# there if I want. And in that case, I'd expect it to behave in the usual way,
# i.e., instances start off with `self.some_state` as a reference to the
# (global) value in the class, but then, as is often done, they assign some
# new value to `self.some_state` at which point it becomes an instance
# variable
some_state = None
def do_something(self, *args, **kwargsd):
# ... do something... then maybe we want to update our internal state:
if self.some_state is None:
self.some_state = 0
else:
self.some_state += 1
t = MyClass()
print(f'{t=}')
print(f'{t.some_state=}')
# crashes if slots=True
t.do_something() # AttributeError: 'MyClass' object attribute 'some_state' is read-only
print(f'{t.some_state=}') |
Also see #971. An improved documentation that stronger points out that slots are being used by default and what possible gotchas might look like would be a good-enough solution for me. |
Your idiom is currently better served by setting a default value and leave it an instance variable. I remember the class-to-instance variable pattern from the before times (aka class variables become magically instance variables), but I don't think it's a good pattern. It relies on implicit behavior people may not be aware of (given that 99.9% of Python programmers are quite new…). IOW your example would be better with @define
class MyClass:
some_state: int | None = None |
closing in the hope of having explained myself sufficiently; discussing the docs issue further in #971 |
I recently started using the
python-attrs
library for the first time, and I think it's really neat. As a new user of it, the only thing that has stood out to me as questionable about its design is thatattrs.define
defaults toslots=True
, so that slotted classes are created by default. I'm curious if anyone else agrees with me that this is a bad default, or what the counterargument is.It think it's bad because it totally changes the semantics of OOP unexpectedly... inheritance is often broken by slots (even without using multiple inheritance), attributes cannot be added to an instance after instantiation (unlike ordinary classes), attributes can become unexpectedly read-only, and there are still more gotchas as enumerated in the Glossary section of the documentation. As a new user who was sold on trying
attrs
by its Philosophy (in the documentation) of "it's about regular classes," "be light on API impact," and "no surprises," these unexpected changes to the core semantics of classes are confusing and annoying.The Glossary even puts it better than I can...
... so why is it the default in the "new API?" (And by the way, the gotchas are almost guaranteed to be surprising, not merely "possibly surprising," given how late in the documentation the discussion of slots appears.)
It seems to me an imbalanced trade-off: violating three pillars of the project Philosophy in exchange for "slightly faster" defaults.
The text was updated successfully, but these errors were encountered: