Skip to content
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

Attrs deletes any non-None class attributes #523

Closed
MarSoft opened this issue Mar 30, 2019 · 2 comments · Fixed by #556
Closed

Attrs deletes any non-None class attributes #523

MarSoft opened this issue Mar 30, 2019 · 2 comments · Fixed by #556
Labels

Comments

@MarSoft
Copy link

@MarSoft MarSoft commented Mar 30, 2019

Currently, attrs will delete any non-None class attributes when _ClassBuilder._delete_attribs is True (i.e. when attributes are specified via class attributes, not directly via these field).
According to comment on _make.py:502, this is intended to remove attr.ib instances from the class.
But it does not play well with attributes declared using PEP 484 type annotations and auto_attribs=True argument to attr.s, because any attribute with non-None default value specified will be pruned from the class, while attributes with None value are retained.
This leads to unexpected behaviour in many cases, in particular when generating documentation using Sphinx autosummary extension which relies on class introspection.
I think we should remove only attr.ib instances, not any not-None fields. At least in auto_attribs mode. Or at least fix the comment.

Attrs version checked: 19.1.0
Code to reproduce:

import attr
import typing

@attr.s(auto_attribs=True)
class Foo:
    bar: int = 42
    baz: typing.Any = None

print([f for f in dir(Foo) if not f.startswith('_')])  # ['baz']
print(Foo.baz)  # 42
print(Foo.bar)  # AttributeError
@hynek
Copy link
Member

@hynek hynek commented Apr 6, 2019

Gosh I'm traveling but isn't the problem just that we need to use a proper/unique sentinel value instead of None here?!

@wsanchez wsanchez added the Bug label May 3, 2019
@hynek
Copy link
Member

@hynek hynek commented Jul 21, 2019

There is totally a bug here, because None and non-None fields should obviously be treated differently.

Removing only attr.ibs would work, however the values those fields have are semantically supposed to be default values that get assigned on instantiation, not class variables. It gets even more complicated when we introduce default factories. We'd have to remove them too…but that'd be highly inconsistent?

So the fix here is to remove None too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants