Add auto_attribs

[hynek]

Well, here we go. I’ve also added examples for the type argument.

Fixes #262

Final step before 17.3.0.

[codecov]

Codecov Report

Merging #277 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #277   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           9      9           
  Lines         666    688   +22     
  Branches      138    145    +7     
=====================================
+ Hits          666    688   +22

Impacted Files	Coverage Δ
src/attr/exceptions.py	100% <100%> (ø)	⬆️
src/attr/_make.py	100% <100%> (ø)	⬆️
src/attr/__init__.py	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a50c4b...5decaa6. Read the comment docs.

[hynek]

Also I wonder if we should troll a bit and add a dataclass = attr.s(auto_attribs=True) alias. ;)

[hynek]

People who might be interested to give some feedback on the API because they expressed interest in this stuff before: @agronholm @wsanchez @ambv @DRMacIver @chadrik

I’m not as afraid to ship a bug as locking us into a conceptually wrong direction.

[wsanchez]

The auto_attrib example looks great to me.

[wsanchez]

The dataclass trolling alias is kind of a great idea but it really wants to be compatible, so this example from PEP 557 should work, for example:

@attr.s(auto_attribs=True)
class InventoryItem:
    quantity_on_hand: int = 0

[agronholm]

Is my hunch correct that it does not properly handle class variable annotations (ClassVar[....])? Those must be ignored for this to work properly.

[hynek]

Is my hunch correct that it does not properly handle class variable annotations (ClassVar[....])? Those must be ignored for this to work properly.

Could you give a full example of what wouldn’t work please?

[hynek]

OMG @wsanchez thanks for reminding me! Implemented in 0ea9b00 and it looks epic!

This works now as expected:

@attr.s(auto_attribs=True)
class C:
    a: int
    x: typing.List[int] = attr.Factory(list)
    y: int = 2
    z: int = attr.ib(default=3)
    foo: typing.Any = None

[agronholm]

Could you give a full example of what wouldn’t work please?

@attr.s(auto_attribs=True)
class Foo:
    var: ClassVar[int]
    foo: str
    bar: bool

Here foo and bar should be recognized as attrib()utes but not var.

[hynek]

@agronholm hm that’s interesting in combination with #220.

Doesn’t a ClassVar have to have a value too? We could treat var: ClassVar[int] = 42 like attr.ib(init=False), or am I missing something?

[agronholm]

Doesn’t a ClassVar have to have a value too?

Why? You could always set it later.

We could treat var: ClassVar[int] = 42 like attr.ib(init=False), or am I missing something?

No, because when combining with slots=True, it would add a __slots__ entry for it, which is wrong.

[hynek]

Why? You could always set it later.

Hm yeah I guess. It’s probably not too common (except for subclassing) but it’s possible.

No, because when combining with slots=True, it would add a __slots__ entry for it, which is wrong.

Well yeah, black magic around slots is implied.

---

The big question now is if we can merge this without blocking ourselves from handling #220 nicely and the answer seems to be “no”.

So what would be a good way to support this?

• allow a way to mark attributes as class variables (either x: ClassVar[int] or attr.ib(class_var=True))
• make those always init=False
• for slots:
  • set the attribute to the value on the new class
  • don't add them to __slots__

Missing something?

---

As for next steps, if we can agree on what I’ve just outlined, I’d take the discussion to #220, promote it to blocking for 17.3 and get this merged.

It’s frustrating that 17.3.0 moved into a far future again but I really don’t want to paint myself into a corner with this because I really care about #220.

[agronholm]

What is the benefit of including class variables in the attrs system in the first place?

[hynek]

In non-slots you can overwrite class variables and they become instance variables, see #220 for a discussion. It kind of gives you “free” defaults.

[agronholm]

I've moved the discussion to the other thread.

[hynek]

From the docs:

ClassVar is not a class itself, and should not be used with isinstance() or issubclass(). ClassVar does not change Python runtime behavior, but it can be used by third-party type checkers.

Ehhh…so what is the clean way here? I just went for basically isinstance(a, ClassVar.__class__) which appears to work.

[agronholm]

Another question: how will you deal with foo: int = 4 and slots=True?

[agronholm]

Also, have you considered that the user might be using backports.typing?

[hynek]

For now it’s treated as a default as per documentation:

>>> i = C()

>>> i.foo
4

>>> i.foo = 5

>>> i.foo
5

Are you insinuating we should make them class variables? I feel like it’s more useful this way and if you want a class var, you use a ClassVar. (it also copies PEP 557 semantics)

[hynek]

Also, have you considered that the user might be using backports.typing?

Since we only support annotations on Python 3.6, I see no value in adding that complexity.

[hynek]

Ugh looks like we need to rethink the handling of ClassVar: ericvsmith/dataclasses#61

[hynek]

As for the factory drama: I think I’ll go with option 1 for now unless @Tinche can shed some different light on this.

---

That said, is there some way to make it work with a different function that works somehow differently? x: List[int] = factory(list) just looks too cute to be passed on. :|

[agronholm]

I still don't understand what the actual problem is with x: List[int] = Factory(list).

[Tinche]

Ugh. I say let's go with @chadrik 's option two and allow it. "Useful now" trumps "maybe easier for mypy" later.

[hynek]

Allowing was never the question, since we just apply the default…it works by accident. :) The question is what we put into the docs. :|

---

But I really need to do something clever about the typing.ClassVar thingie for the reason alone, that typing imports are slow. Sigh

[chadrik]

That said, is there some way to make it work with a different function that works somehow differently?

There is a fourth option, which I did not see previously because I was blinded by my own bias. We can stub Factory so that it returns Any, which disables type checking for this object completely and would therefore allow it to be used in the short form or the long form without static type check errors. The downside is that you are no longer warned about actual errors, such as:

x: typing.List[int] = attr.Factory(str)

I prefer option 2 over this. With option 2 there is a low chance of false errors, but with option 4 there is a high chance of undetected errors.

[wsanchez]

@chadrik: I agree.

The false errors in option 2 are rare and easily worked around with a cast.

[wsanchez]

Actually, the more I think about it, the more Option 2 seems correct to me. x is a List[int] in the intended API for class C. That Factory is involved at all is only because I'm using attrs; that is: Factory is an artifact of attrs's API, not of C's, so C's annotations ideally shouldn't leak that out.

No?

[agronholm]

Actually, the more I think about it, the more Option 2 seems correct to me. x is a List[int] in the intended API for class C. That Factory is involved at all is only because I'm using attrs; that is: Factory is an artifact of attrs's API, not of C's, so C's annotations ideally shouldn't leak that out.

I just tried, and PyCharm does not make the type of x the union of List[int] and Factory. MyPy does. But if Factory() was marked as returning object, it would not pollute the type information, right?

[chadrik]

I just tried, and PyCharm does not make the type of x the union of List[int] and Factory. MyPy does.

I'm not entirely sure what you mean. Without any special pyi stubs for attrs, this is an error in mypy:

x: typing.List[int] = attr.Factory(list)

Based on PEP484, when you annotate a variable with a type, the value assigned to it must be "compatible" with that type. Nothing here becomes the union of anything else. A Factory is not a list, so it's an error. I'm not sure what PyCharm does here, but it still has a lot of catching up to do wrt PEP484.

But if Factory() was marked as returning object, it would not pollute the type information, right?

Still not sure what you mean. If you want the above code to work without an error in mypy, Factory (or the stubs for Factory) must return a value that is compatible with List[int]. Any is compatible with everything (it's the union of all possibilities), so as I said earlier, that is one option, but not one that I like. object is the intersection of all possibilities: i.e. the lowest common denominator. Only operations that are possible on all types are valid with object. It's not what you want here.

[agronholm]

Still not sure what you mean. If you want the above code to work without an error in mypy, Factory must return a value that is compatible with List[int].

Could you show me a simple script with such an annotation that would result in a mypy error on inspection?

[chadrik]

Could you show me a simple script with such an annotation that would result in a mypy error on inspection?

import typing
import attr
x: typing.List[int] = attr.Factory(list)

Make sure that you set MYPYPATH to the <attrs_repo>/src directory (do not set it to your site-packages directory).

[agronholm]

Wait, are you saying that I need to have a checked out copy of the attrs git repo whenever I run mypy against my code??

[hynek]

That or get the stubs into typeshed IIUC. It’s kind of bonkers.

[agronholm]

Alright, no wonder I didn't get any errors before then.

[chadrik]

That or get the stubs into typeshed IIUC. It’s kind of bonkers.

Yeah, it's very annoying, but there's a pep in progress to solve it at the distutils/setuptools level.

Alright, no wonder I didn't get any errors before then.

Well, you would have gotten an error like Cannot find module named 'attr'

[agronholm]

Well, you would have gotten an error like Cannot find module named 'attr'

I routinely skip missing imports with MyPy because most libraries don't have stubs.

[hynek]

Since everyone seems to love option 2...do I have to do something a out it or am I done?

[chadrik]

Since everyone seems to love option 2...do I have to do something a out it or am I done?

I handled it over on #238

[chadrik]

Since everyone seems to love option 2...do I have to do something a out it or am I done?

I handled it over on #238