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

Inherited attributes of subclasses no longer compare equal to the equivalent attributes of base class #682

Closed
pganssle opened this issue Sep 1, 2020 · 2 comments · Fixed by #684

Comments

@pganssle
Copy link
Member

pganssle commented Sep 1, 2020

A change between 19.3.0 → 20.1.0 which doesn't seem to be deliberate, but may be related to #635 is that inherited attributes no longer compare equal to the attributes of the base class. Here is a MWE:

import attr

@attr.s()
class BaseClass(object):
    x = attr.ib()

@attr.s()
class SubClass(BaseClass):
    pass

assert attr.fields(BaseClass)[0].name == attr.fields(SubClass)[0].name   # Succeeds in all versions
assert attr.fields(BaseClass)[0] == attr.fields(SubClass)[0]  # Fails in 20.1.0

Interestingly, if you shadow the attribute, it works as expected:

import attr

@attr.s()
class BaseClass(object):
    x = attr.ib()

@attr.s()
class SubClass(BaseClass):
    x = attr.ib()

# Works in all versions
assert attr.fields(BaseClass)[0].name == attr.fields(SubClass)[0].name
assert attr.fields(BaseClass)[0] == attr.fields(SubClass)[0]

I think the issue is that inherited is included in the calculation of whether two Attributes are equal or not.

I discovered this when upgrading some code that does something like this:

import attr

@attr.s()
class BaseClass:
    x = attr.ib()

@attr.s()
class SubClass(BaseClass):
    y = attr.ib()

def f(x) -> None:
    print(x)

def my_function(obj : BaseClass) -> None:
    kwargs = attr.asdict(obj, filter=attr.filters.include(*attr.fields(BaseClass)))
    f(**kwargs)

if __name__ == "__main__":
    my_function(SubClass(1, 2))

The idea there is that they want to pull out the subset of the structure that corresponds to the base class.

In this case, I can imagine working around it by pulling out the name attribute for each of the Attributes (since in this case I think they would want the subclass to pass along any attribute with the name of an attribute on the base class, not just ones that are configured the same way). I can imagine there might be other problems, though, if attr.filters.include(*attr.fields(BaseClass)) is a common pattern.

@hynek
Copy link
Member

hynek commented Sep 2, 2020

Yeah for all practical purposes, I think we've got it wrong ATM. And I think hashing should also not take it into account. Technically it's wrong but in practice I think it makes it useless.

@hynek
Copy link
Member

hynek commented Sep 2, 2020

OK, pls check #684

hynek added a commit that referenced this issue Sep 2, 2020
hynek added a commit that referenced this issue Sep 3, 2020
@hynek hynek closed this as completed in #684 Sep 3, 2020
hynek added a commit that referenced this issue Sep 3, 2020
* Ignore inherited field when comparing Attributes

fixes #682

* add newsfragment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants