Feedback to next generation APIs

[hynek]

This is the issue to discuss the defaults and behavior of attr.define(), attr.mutable(), and attr.frozen() before they are finalized inside the new attrs namespace.

Feedback that we find justified will be spun out into separate issues.

Known issues:

• mypy doesn't know about the new-style APIs yet and there's nothing we can do about it. However you can use this mypy plugin for now (for more details see https://www.attrs.org/en/stable/extending.html#wrapping-the-decorator). PR is submitted in Add support for next-gen attrs API python/mypy#9396.
• __eq__ and __ne__ aren't correctly auto-detected (fixed) @attr.define fails to auto-detect __eq__ #670
• Unannotated attributes fail if define is called with arguments/parantheses (fixed) Hybrid behavior doesn't work when maybe_cls=None and no annotations #673
• Inheriting from frozen classes requires to manually set on_setattr to None (fixed). Feedback to next generation APIs #668 (comment))

[rouge8]

Overall I love the new APIs. When updating some code to use @attr.define instead of @attr.s(auto_attribs=True), I was surprised by one behavior of slotted classes -- you don't seem to be able to use unittest.mock.patch.object() on methods on instances of slotted classes. It's not a result of anything in attrs (in the code snippet below I don't use attrs at all), but it might be worth noting in the docs since making slots=True the default behavior means attrs will be how many people encounter this for the first time.

import unittest.mock

class Slotted:
    __slots__ = ()

    def method(self):
        return 1

s = Slotted()
assert s.method() == 1

with unittest.mock.patch.object(s, "method", return_value=3):
    assert s.method() == 3

# Errors with:
# Traceback (most recent call last):
#   File "/usr/local/Cellar/python@3.8/3.8.5/Frameworks/Python.framework/Versions/3.8/lib/python3.8/unittest/mock.py", line 1490, in __enter__
#     setattr(self.target, self.attribute, new_attr)
# AttributeError: 'Slotted' object attribute 'method' is read-only

[hynek]

Ah yeah, thanks for that reminder, I will add it to the glossary!

[euresti]

Hmm. I'm not sure I like the hybrid behavior.

Imagine I forget to annotate something. Now instead of getting a nice error at import time I have to wait until I try to create something.

from attr import define, field

@define()
class A:
    x: int
    y = field(8)

Oh this case is actually pretty bad because I might not catch it even if i do A(8)

[hynek]

I thought about this for a while too.

I decided it’s worth the comfort because the only possible confusing error state is the one you describe and it’s impossible to miss even short-term when an attribute is missing. I’m open to being convinced otherwise but ISTM that in practice this trade-off is worth it. 🤔

[hynek]

How about the following: auto_attrib=None = Hybrid, everything else means what it means now?

[euresti]

What's the default value? (I suggest auto_attribs=True but that's my personal preference because I always write type annotated code)

I'd also like it if auto_attrib=None would raise if it found a mixture of attribs and annotations. Is it ok to add this to _transform_attrs or is affecting attr.s not something we should do?

[hynek]

What's the default value? (I suggest auto_attribs=True but that's my personal preference because I always write type annotated code)

I'd also like it if auto_attrib=None would raise if it found a mixture of attribs and annotations. Is it ok to add this to _transform_attrs or is affecting attr.s not something we should do?

Well None would be default which means “guess”. I’m Camp True too, but I don’t want to punish the typing haters for whine attrs is currently the only refuge.

I don’t thing an error on a mix of attr.ib and annotations is feasible because there a valid use cases for that. Maybe raise an error but add an option to override it?

[euresti]

I don't really see a good use for this:

@define()
class A:
    x: int
    y = field(8)

A.x won't exist and you won't be able to add it because of __slots__. Worst of all, mypy will think it exists.

Perhaps this could be ok:

@define()
class A:
    x: int = 72
    y = field(8)

But one could argue that it should be:

@define()
class A:
    x: ClassVar[int] = 72
    y = field(8)

Because it's gonna be a Class attribute not an instance one.

[euresti]

Oh and thinking about it more auto_attribs=None is a fine default value. Because if I use annotations they'll work the way I want them. (i.e. I don't have to add auto_attribs=True everywhere.

[hynek]

To be clear: it’s 100% not gonna be necessary to pass slots=True or auto_attribs=True because that’s my settings. 😅 The q now is how to handle your scenario. Truth to be told we can raise an error and when someone complains, we can still add an option to allow for it?

[euresti]

That option could be auto_attribs=False. :) Or did you want some kind of global disable?

[hynek]

God No 😅! I’m just trying to find a way where the common use case is not passing anything and it just works – ideally without exposing the users to sharp edges.