attrs
attrs copied to clipboard
Add `reuse()` to `Attribute` for field evolution
Summary
Preliminary implementation of "evolving" existing Attribute instances back into _CountingAttr instances, so that users may (easily) reuse attribute definitions from already defined classes. Should resolve #637, #698, #829, #876, #946, and #1424.
Usage:
@attrs.define
class Parent:
a: int = 100
@attrs.define
class Child:
a = attrs.fields(Parent).a.evolve(default=200).to_field()
# Now that a is in scope of Child, we can attach methods to it like normal:
@a.validator
def a_validator(...):
...
assert attrs.fields(Child).a.type is int
assert Child().a == 200
This syntax is verbose, but least magical. And because you manually specify which class you want to pull attributes from, inheritance is not required; you can compose new classes entirely from parts of existing classes regardless of structure:
@attrs.define
class A:
a: int = 1
@attrs.define
class B:
b: int = 2
@attrs.define
class Composite:
a = attrs.fields(A).a.evolve(...).to_field()
b = attrs.fields(B).b.evolve(...).to_field()
Utility methods like attrs.make_class() and the these kwarg in attrs.define() also work as you would expect.
One potential pain point with a simple implementation of this feature is inherited attributes being reordered when redefined in this manner:
@attr.s
class BaseClass:
x: int = attr.ib(default=1)
y: int = attr.ib(default=2)
@attr.s
class SubClass(BaseClass):
x = attr.fields(BaseClass).x.evolve(default=3).to_field()
# Because x was redefined, it appears after y
assert "SubClass(y=2, x=3)" == repr(SubClass())
In my opinion, this behavior is a failure of the implementation, as it is reasonable to expect users to want to preserve this ordering, as in a worst-case scenario it can lead to non-constructable classes. This PR implements a new bool keyword argument inherited to field, which tells attrs to use the ordering of the field in the parent class as opposed to adding to the end:
@attr.s
class SubClass(BaseClass):
x = attr.fields(BaseClass).x.evolve(default=3, inherited=True).to_field()
assert "SubClass(x=3, y=2)" == repr(SubClass())
Criticisms of this inherited keyword are welcome, as I'm not convinced it is the best solution. However, I do think this functionality needs to be present in attrs in order to make this kind of attribute evolution a tenable approach.
Pull Request Check List
- [x] Do not open pull requests from your
mainbranch – use a separate branch! - [x] Added tests for changed code.
- [ ] New features have been added to our Hypothesis testing strategy.
- [ ] Changes or additions to public APIs are reflected in our type stubs (files ending in
.pyi).- [ ] ...and used in the stub test file
tests/typing_example.py. - [ ] If they've been added to
attr/__init__.pyi, they've also been re-imported inattrs/__init__.pyi.
- [ ] ...and used in the stub test file
- [x] Updated documentation for changed code.
- [x] New functions/classes have to be added to
docs/api.rstby hand. - [x] Changes to the signatures of
@attr.s()and@attrs.define()have to be added by hand too. - [x] Changed/added classes/methods/functions have appropriate
versionadded,versionchanged, ordeprecateddirectives. The next version is the second number in the current release + 1. The first number represents the current year. So if the current version on PyPI is 22.2.0, the next version is gonna be 22.3.0. If the next version is the first in the new year, it'll be 23.1.0.- [x] If something changed that affects both
attrs.define()andattr.s(), you have to add version directives to both.
- [x] If something changed that affects both
- [x] New functions/classes have to be added to
- [x] Documentation in
.rstand.mdfiles is written using semantic newlines. - [x] Changes (and possible deprecations) have news fragments in
changelog.d. - [x] Consider granting push permissions to the PR branch, so maintainers can fix minor issues themselves without pestering you.
Why does it need to be attrs.fields(Parent).a.evolve(default=200).to_field() instead of maybe attrs.fields(Parent).a.to_field(default=200)? We'll need to add Mypy support to this before it can be used for real, and we should look at making that as easy as possible.
If the goal is to reduce function chains, then how about the keyword reuse()? That might be more descriptive in this context than to_field():
class A:
x = attrs.fields(Example).x.reuse()
y = attrs.fields(Example).y.reuse(default="whatever")
_z = attrs.fields(OtherExample).a.reuse(alias="z")
I like it!
What exactly should I annotate for mypy? I notice that neither the return result of attr.fields() nor _CountingAttr itself have annotations already:
https://github.com/python-attrs/attrs/blob/755b1778afcee1a323d8efbd25df73da236ab4ab/src/attr/init.pyi#L311
Should I just try to annotate everything this new feature touches? The contribution guidelines are a bit sparse in this regard.
So y = attrs.fields(Example).y.reuse(default="whatever") is cool indeed – I'm just not sure if that will ever be possible to get into typing at all??
I'm kinda OK with saying that you have to redefine your types, otherwise it gets hairy with ordering anyways (or some magic helper type, e.g., y: Annotated[Inherit] = attrs.fields(Example).y.reuse(default="whatever")).
But how could any of this be made type-safe? y: Annotated[Inherit(attrs.fields(Example).y, default="whatever")] would be probably even cooler since it circumvents the ordering problems, but I guess we should think ahead here? Does maybe Pydantic have something similar so we could join PEP forces or something?
We could contribute work to the mypy plugin so it would work 😇
I'm kinda OK with saying that you have to redefine your types, otherwise it gets hairy with ordering anyways (or some magic helper type, e.g., y: Annotated[Inherit] = attrs.fields(Example).y.reuse(default="whatever")).
This would make sense. The only reason it is omitted in this particular implementation is because the type is already inherited from the reuse() call and (re)including a type annotation would be akin to defining it twice, which attrs complains about. This can simply be rewritten to not inherit the type, and to default to Any if no type annotation is provided.
y: Annotated[Inherit(attrs.fields(Example).y, default="whatever")]would be probably even cooler since it circumvents the ordering problems
This is not a bad idea. If I understand you correctly, does that make the proposed syntax:
@attrs.define
class A:
x: int = 10
y: str = attrs.field(default="blah")
@attrs.define
class B:
z: dict = {"test": "result"}
@attrs.define
class Child(B, A):
x: Annotated[Inherited(attrs.fields(A).x, default=20)] # Preserve parent ordering
y: str = attrs.fields(A).y.reuse() # Add to end after inherited
z: Annotated[Inherited(attrs.fields(B).z, factory=dict)] # Preserve parent ordering
assert "Child(x=20, z={}, y='blah')" == repr(Child())
It seems the question of what order to put inherited fields in is not trivial. I suppose you could argue that if you need absolute control of field order, then you should create an entirely composite class using reuse() and ignore inheritance entirely, which violates DRY a little but keeps things obvious.
We could contribute work to the mypy plugin so it would work 😇
I'll look into this. I assume you're referring to this?