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

AttributeError propagation is broken in cached_property with slots=True #1230

Closed
n-splv opened this issue Jan 31, 2024 · 3 comments · Fixed by #1253
Closed

AttributeError propagation is broken in cached_property with slots=True #1230

n-splv opened this issue Jan 31, 2024 · 3 comments · Fixed by #1253
Labels

Comments

@n-splv
Copy link

n-splv commented Jan 31, 2024

Let's start with a regular class:

class A:

    @cached_property
    def x(self):
        return self.y

    @property
    def y(self):
        raise AttributeError("Message")
        
a = A()
print(a.x)  # AttributeError: Message

Just as expected. Now let's use attrs with __dict__:

@define(slots=False)
class A:
    ...

print(a.x)  # AttributeError: Message

So far so good. But:

@define  # slots=True by default
class A:
    ...

print(a.x)  # AttributeError: 'A' object has no attribute 'y'

This made me spend quite some time wondering where did my attribute go :)

@hynek
Copy link
Member

hynek commented Jan 31, 2024

I'm a little confused about what's going on here – are we somehow rewriting AttributErrors?

ping @diabolo-dan 😇

@hynek hynek added the Bug label Jan 31, 2024
@diabolo-dan
Copy link
Contributor

Yeah, as I recall there's some rewriting there to avoid a confusing __getattr__ attribute error.

I think there's a try/except that could be rewritten as an if hasatr, which would probably resolve this. There are some performance implications to the change, but it's probably a reasonable tradeoff for optimising the more common case.

I won't be able to look at it this week, as I'm on holiday, but might be able to take a look when I'm back next week.

@hynek
Copy link
Member

hynek commented Jan 31, 2024

Thanks, and enjoy your holiday!

dlax added a commit to dlax/attrs that referenced this issue Mar 5, 2024
In slotted classes' generated __getattr__(), we try __getattribute__()
before __getattr__(), if available, and eventually let AttributeError
propagate. This matches better with the behaviour described in Python's
documentation "Customizing attribute access":

  https://docs.python.org/3/reference/datamodel.html#customizing-attribute-access

Fix python-attrs#1230
dlax added a commit to dlax/attrs that referenced this issue Mar 5, 2024
In slotted classes' generated __getattr__(), we try __getattribute__()
before __getattr__(), if available, and eventually let AttributeError
propagate. This matches better with the behaviour described in Python's
documentation "Customizing attribute access":

  https://docs.python.org/3/reference/datamodel.html#customizing-attribute-access

Fix python-attrs#1230
github-merge-queue bot pushed a commit that referenced this issue Apr 2, 2024
* Preserve AttributeError in slotted classes with cached_property

In slotted classes' generated __getattr__(), we try __getattribute__()
before __getattr__(), if available, and eventually let AttributeError
propagate. This matches better with the behaviour described in Python's
documentation "Customizing attribute access":

  https://docs.python.org/3/reference/datamodel.html#customizing-attribute-access

Fix #1230

* Update changelog.d/1253.change.md

---------

Co-authored-by: Hynek Schlawack <hs@ox.cx>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants