Skip to content

Commit c0d42e9

Browse files
Pranav Mishraclaude
andcommitted
Match filter attributes by identity instead of equality
attrs.filters.include/exclude stored the passed Attribute instances in a frozenset and tested membership with `attribute in attrs`. Attribute equality ignores the owning class, so two identically-configured attributes on different classes compare equal - filtering by one class's attribute also matched the other class's. Keep the attributes in a tuple and match them by identity instead. Field-name strings still match by name across classes, which covers the cross-class use case. Fixes #864 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 45de9be commit c0d42e9

3 files changed

Lines changed: 50 additions & 5 deletions

File tree

changelog.d/864.change.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
`attrs.filters.include` and `attrs.filters.exclude` now match `attrs.Attribute` instances by identity instead of equality, so filtering by an attribute of one class no longer also matches an identically-configured attribute of a different class.

src/attr/filters.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,18 @@
99

1010
def _split_what(what):
1111
"""
12-
Returns a tuple of `frozenset`s of classes and attributes.
12+
Returns a `frozenset` of classes, a `frozenset` of names, and a `tuple` of
13+
attributes.
14+
15+
Attributes are kept as a tuple and matched by identity rather than put in a
16+
`frozenset`: `Attribute` equality ignores the owning class, so two
17+
identically-configured attributes on different classes compare equal and
18+
set membership would match the wrong one (see #864).
1319
"""
1420
return (
1521
frozenset(cls for cls in what if isinstance(cls, type)),
1622
frozenset(cls for cls in what if isinstance(cls, str)),
17-
frozenset(cls for cls in what if isinstance(cls, Attribute)),
23+
tuple(cls for cls in what if isinstance(cls, Attribute)),
1824
)
1925

2026

@@ -39,7 +45,7 @@ def include_(attribute, value):
3945
return (
4046
value.__class__ in cls
4147
or attribute.name in names
42-
or attribute in attrs
48+
or any(attribute is a for a in attrs)
4349
)
4450

4551
return include_
@@ -66,7 +72,7 @@ def exclude_(attribute, value):
6672
return not (
6773
value.__class__ in cls
6874
or attribute.name in names
69-
or attribute in attrs
75+
or any(attribute is a for a in attrs)
7076
)
7177

7278
return exclude_

tests/test_filters.py

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ def test_splits(self):
3030
assert (
3131
frozenset((int, str)),
3232
frozenset(("abcd", "123")),
33-
frozenset((fields(C).a,)),
33+
(fields(C).a,),
3434
) == _split_what((str, "123", fields(C).a, int, "abcd"))
3535

3636

@@ -79,6 +79,24 @@ def test_drop_class(self, incl, value):
7979
i = include(*incl)
8080
assert i(fields(C).a, value) is False
8181

82+
def test_matches_attribute_by_identity(self):
83+
"""
84+
An identically-configured attribute on a different class is not
85+
included: attributes are matched by identity, not equality (#864).
86+
"""
87+
88+
@attr.s
89+
class D:
90+
a = attr.ib()
91+
b = attr.ib()
92+
93+
assert fields(C).a == fields(D).a
94+
assert fields(C).a is not fields(D).a
95+
96+
i = include(fields(C).a)
97+
assert i(fields(C).a, 42) is True
98+
assert i(fields(D).a, 42) is False
99+
82100

83101
class TestExclude:
84102
"""
@@ -124,3 +142,23 @@ def test_drop_class(self, excl, value):
124142
"""
125143
e = exclude(*excl)
126144
assert e(fields(C).a, value) is False
145+
146+
def test_matches_attribute_by_identity(self):
147+
"""
148+
An identically-configured attribute on a different class is not
149+
excluded: attributes are matched by identity, not equality (#864).
150+
"""
151+
152+
@attr.s
153+
class D:
154+
a = attr.ib()
155+
b = attr.ib()
156+
157+
assert fields(C).a == fields(D).a
158+
assert fields(C).a is not fields(D).a
159+
160+
e = exclude(fields(C).a)
161+
# C.a is excluded ...
162+
assert e(fields(C).a, 42) is False
163+
# ... but the equal-but-distinct D.a is not.
164+
assert e(fields(D).a, 42) is True

0 commit comments

Comments
 (0)