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

Possible lazy=True Feature? #573

Open
pymarv opened this issue Sep 16, 2019 · 14 comments
Open

Possible lazy=True Feature? #573

pymarv opened this issue Sep 16, 2019 · 14 comments
Labels

Comments

@pymarv
Copy link

pymarv commented Sep 16, 2019

Hi Everyone,

First, thanks for the great tool -- I really love writing Python code with it!

I find myself doing things like this a lot when I want b and c to only run when called (and then only once with the result cached).

import attr
from cached_property import cached_property

@attr.s
class MyClass:
    a = attr.ib(
        validator=attr.validators.instance_of(int),
        converter=int,
    )

    @cached_property
    def b(self):
        return str(self.a) * 3

    @cached_property
    def c(self):
        return int(self.b) * 3

foo = MyClass(a='1')
print(f'foo.a: {foo.a}')
print(f'foo.b: {foo.b}')
print(f'foo.c: {foo.c}')

But then I have to "break out of attrs" and loose the other goodness it provides. I know I can do something like this:

import attr

@attr.s
class MyClass:
    a = attr.ib(
        validator=attr.validators.instance_of(int),
        converter=int,
    )
    b = attr.ib(
        validator=attr.validators.instance_of(str),
        default=attr.Factory(
            takes_self=True,
            factory=lambda self: str(self.a) * 3,
        )
    )
    c = attr.ib(
        validator=attr.validators.instance_of(int),
        default=attr.Factory(
            takes_self=True,
            factory=lambda self: int(self.b) * 3,
        )
    )

foo = MyClass(a='1')
print(f'foo.a: {foo.a}')
print(f'foo.b: {foo.b}')
print(f'foo.c: {foo.c}')

But then b and c get "run" or "built" every time, even if they aren't needed (and I'm obviously assuming in reality they do more work they my toy example here shows).

What I really want to do is something like:

import attr

@attr.s
class MyClass:
    a = attr.ib(
        validator=attr.validators.instance_of(int),
        converter=int,
    )
    b = attr.ib(
        validator=attr.validators.instance_of(str),
        lazy=True,
        default=attr.Factory(
            takes_self=True,
            factory=lambda self: str(self.a) * 3,
        )
    )
    c = attr.ib(
        validator=attr.validators.instance_of(int),
        lazy=True,
        default=attr.Factory(
            takes_self=True,
            factory=lambda self: int(self.b) * 3,
        )
    )

foo = MyClass(a='1')
print(f'foo.a: {foo.a}')
print(f'foo.b: {foo.b}')
print(f'foo.c: {foo.c}')

Or even better, something that doesn't require me to specify takes_self=True every time... maybe something like:

import attr

@attr.s
class MyClass:
    a = attr.ib(
        validator=attr.validators.instance_of(int),
        converter=int,
    )
    b = attr.ib(
        validator=attr.validators.instance_of(str),
        lazy=True,
        builder=attr.Builder(
            factory=lambda self: str(self.a) * 3,
        )
    )
    c = attr.ib(
        validator=attr.validators.instance_of(int),
        lazy=True,
        builder=attr.Builder(
            factory=lambda self: int(self.b) * 3,
        )
    )

foo = MyClass(a='1')
print(f'foo.a: {foo.a}')
print(f'foo.b: {foo.b}')
print(f'foo.c: {foo.c}')

Thoughts on if something like this would be possible? I haven't dug into guts of attrs yet enough to know if it's doable, but it seems like it would be a great enhancement if something like that is possible. I came not too long ago from a non-Python environment where we built classes almost exactly like I show in my last example above, and it's a super efficient and expressive way to do things.

Thanks!

@RonnyPfannschmidt
Copy link

It's absolutely unclear how to map a straightforward simple thing like plain cached attributes into something matching declared attributes mixed with computation

@hynek
Copy link
Member

hynek commented Sep 16, 2019

I think if anything this falls under the greater topic of treating properties as first-class citizens.

@pymarv
Copy link
Author

pymarv commented Sep 16, 2019

OK thanks. So it's basically file it under the heading of "might be nice but not doable", right?

Anyone know any alternatives to what I'm doing with @cached_property? Or is that likely the best bet (at least until Python 3.8 where I guess it is supposed to be built in)?

If the lazy=True isn't doable, do any others see value in something like builder= that would equivalent to attr.Factory(takes_self=True)?

@hynek
Copy link
Member

hynek commented Sep 17, 2019

Something might be doable when we start working on first class property support (that I'd like to have myself, but there's more pressing work in attrs right now). But I can't give you an answer from the top of my head.

@pymarv
Copy link
Author

pymarv commented Sep 17, 2019

OK, thank you very much for the response and consideration.

@pymarv
Copy link
Author

pymarv commented Apr 6, 2020

@pwwang Thanks! Not sure how I missed that when you posted it! Apologies for the delay. Any thoughts on if this is "production ready"?

@hynek & @RonnyPfannschmidt Would any what pwwang work for some of the desired/planned property stuff? I didn't know if there was a way to incorporate that as a starting point. I'm only digging into attrs internals recently, but I'm willing to help if I can.

Thanks

@hynek
Copy link
Member

hynek commented Apr 18, 2020

Currently the main focus of mine (if you can talk about focus in these times…) is to finish up the work around import attrs. First class property support is on my roadmap since it annoys me too, but the road is very long alas.

@Drino
Copy link
Contributor

Drino commented Oct 29, 2020

Hello! :)

I have a little bit different issues, though they also can be solved with @cached_property-like factory.

UPD: I was wrong, I've found out that I can redefine all attributes of interest in my child class and then allow any defaults logic I'd like. Though, I still find the half-initialized self pretty confusing and think that having a lazy factory based on cached_property is a nice approach to avoid facing such issues (e.g. #707).

I've found myself writing class hierarchies like this:

@attr.dataclass(frozen=True)
class Parent:
    id: int
    ... # Some other fields here


@attr.dataclass(frozen=True)
class _MagicChild(Parent):
    id: int = attr.ib(init=False)
    magic_number: int


@attr.dataclass(frozen=True)
class MagicChild(_MagicChild):
    @cached_property
    def id(self) -> int:
        return self.magic_number * 42

This trick with extra-class is required, as I can't use magic_number in id attribute factory (as it works with half-initialized self and id is the first attribute in Parent) and I can't define both the @cached_property and attr.ib named id in one class.

This whole machinery allows me to get away from half-initialized self issues to manual attribute dependency management, but looks gross, so I was thinking that maybe it deserves a shortcut.

The API update proposal is following:

  • @default decorator gets lazy option
  • Factory gets lazy option (default is False). I'm not sure if it should imply takes_self=True or not.

Then, child class from my example can be rewritten in following ways

@attr.dataclass(frozen=True)
class MagicChildDefault(Parent):
    id: int = attr.ib(init=False)
    magic_number: int

    @id.default(lazy=True)
    def _id_default(self):
        return 42 * self.magic_number

@attr.dataclass(frozen=True)
class MagicChildFactory(Parent):
    id: int = attr.ib(init=False, default=attr.Factory(lambda self: self.magic_number * 42, takes_self=True, lazy=True))
    magic_number: int

@Drino
Copy link
Contributor

Drino commented Nov 4, 2020

I've though about the design from the previous post and it seems that these "lazy" factories are pretty messy in composition with converters.

  • First point is - we need to store converter in the property __get__ itself...
  • Other point is 3-argument converters. They may trigger "lazy" factory on half-initalized instance even while the value is passed, but not set, making it really messy to debug :(

Considering issues with converters, I assume that the right way of thinking about lazy-initializers is the most low-level - i.e. they are not factories, they are just descriptors that can be defined by user and that will be applied if value is not set (e.g. - if attr.ib(init=False)). It wouldn't make handling half-initialized self simpler, though it is much explicit.

In such approach, my example can be rewritten as:

@attr.s
class MagicChild(Parent):
    @cached_property  # Can be easily re-defined in child
    def id(self):
        return 42 * self.magic_number

    id = attr.ib(init=False, descriptor=id)

There was another request for not setting a value and leaving it to descriptor - #354 . Maybe if an attribute descriptor is set, we can leave the value empty if got attr.NOTHING? Seems like it'll solve both issues here. If somebody needs conversion/validation for descriptor (?) - they can run converter/validator manually.

I've checked attr-property project but it looks very clumsy and doesn't seem to allow usage of existing descriptor.

@hynek
Copy link
Member

hynek commented Jan 19, 2021

Oof November! Sorry for taking me so long – it's one of those classic "this needs a proper response when I have the time". 😂😭


Firstly, I'm glad the lazy Factory approach is off the table because it honestly felt off in my gut, without being able to put the finger on it.

I have to admit that I've never used descriptors in anger, so I have a hard time to comment on this competently. It does seem fair tho to consider it a more low-level construct with the implications you noted.

We could add it as an experimental feature with the caveat that it might change (like attr.define basically) with the possibility of adding more high-level capabilities down the road, if necessary. How complicated do you thing is it gonna be to integrate?

However I'm afraid to ask you for an MVP because I already do have two PRs that cut deep into attrs' bowels and I'm afraid to send you on a constant-rebasing mission. 🤔 IOW: I want to land #731 and #627 before starting new big surgeries inside of _make.py.

@Drino
Copy link
Contributor

Drino commented Jan 19, 2021

Hi, @hynek , thanks for the comment! :)

It seems a pretty simple feature to integrate - I believe I can make some MVP (with a shortcut that puts descriptor into metadata and using field_transformer to set it on the class itself) in an evening. It wouldn't solve non-init case, though, and also would break mypy plugin/PyCharm inspections.

TBH being able to set a classvar with the same name as attr.ib (which is exactly what descriptor is about) seems like a core-level feature to me - I've even created a separate feature-request #746 with some design proposal. If you have some time - PTAL :3 I'd prefer implementing MVP for it than a standalone solution.

@hynek
Copy link
Member

hynek commented Jan 19, 2021

Argl, I thought I'm answering to #746 and wondered why your wording changed.

offbyone added a commit to offbyone/environ-config that referenced this issue Jun 26, 2021
Initializing the SecretsManager as an attr factory caused the client to
be initialized too early to mock it out with moto for unit testing, as
was called out in hynek#24.

Until python-attrs/attrs#573 is fixed, we need to manually lazy-init the
client for SecretsManager
offbyone added a commit to offbyone/environ-config that referenced this issue Jun 27, 2021
Initializing the SecretsManager as an attr factory caused the client to
be initialized too early to mock it out with moto for unit testing, as
was called out in hynek#24.

Until python-attrs/attrs#573 is fixed, we need to manually lazy-init the
client for SecretsManager
hynek pushed a commit to hynek/environ-config that referenced this issue Jun 28, 2021
* testing: allow lazy init of boto client

Initializing the SecretsManager as an attr factory caused the client to
be initialized too early to mock it out with moto for unit testing, as
was called out in #24.

Until python-attrs/attrs#573 is fixed, we need to manually lazy-init the
client for SecretsManager

* 21.3.0 drops Python 2.7 support.

I mean, you _can_ import awssm, but the tests don't work on 2.7 any
more.

Co-authored-by: Chris Rose <[email protected]>
@waszil
Copy link

waszil commented Dec 4, 2021

Hi, maybe something like this can be useful: https://gist.github.com/waszil/171d64e5b94cdd51c404d41332f476c0

@kdebrab
Copy link

kdebrab commented Feb 6, 2022

@pymarv, I think you can achieve the desired behaviour by combining a private attribute with the cached_property:

import attr
from functools import cached_property  # Python>=3.8


@attr.s
class MyClass:
    a = attr.ib(
        validator=attr.validators.instance_of(int),
        converter=int,
    )
    _b = attr.ib(
        validator=attr.validators.optional(attr.validators.instance_of(str)),
        default=None
    )
    _c = attr.ib(
        validator=attr.validators.optional(attr.validators.instance_of(int)),
        default=None
    )

    @cached_property
    def b(self):
        if self._b is None:
            return str(self.a) * 3
        else:
            return self._b

    @cached_property
    def c(self):
        if self._c is None:
            return int(self.b) * 3
        else:
            return self._c

foo = MyClass(a='1')
print(f'foo.a: {foo.a}')
print(f'foo.b: {foo.b}')
print(f'foo.c: {foo.c}')

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

No branches or pull requests

7 participants