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

Incorrect treatment of Final class variables when auto_attribs=True #784

Closed
dargueta opened this issue Mar 22, 2021 · 12 comments
Closed

Incorrect treatment of Final class variables when auto_attribs=True #784

dargueta opened this issue Mar 22, 2021 · 12 comments
Labels
Typing Typing/stub/Mypy/PyRight related bugs.

Comments

@dargueta
Copy link
Contributor

dargueta commented Mar 22, 2021

attrs appears to treat an initialized final class variable as an instance variable. I'd expect that this would follow PEP 591, specifically this sentence in the "Semantics and Examples" section:

Type checkers should infer a final attribute that is initialized in a class body as being a class variable. Variables should not be annotated with both ClassVar and Final.

However, that doesn't appear to be the case. Suppose we have the following code, where x is intended to be an "immutable" class variable and y is an instance variable:

@attr.s(auto_attribs=True)
class A:
    x: Final[int] = 123
    y: int

The code crashes with an error:

ValueError: No mandatory attributes allowed after an attribute with a default value or factory.  Attribute in question: Attribute(name='y', default=NOTHING, validator=None, repr=True, eq=True, order=True, hash=None, init=True, metadata=mappingproxy({}), type=<class 'int'>, converter=None, kw_only=False, inherited=False, on_setattr=None)

This indicates that x isn't being properly treated as a class variable and skipped over. Indeed, if we reverse the order of the declarations, it "works" in the sense that it doesn't crash, but it generates a class with incorrect behavior.

>>> @attr.s(auto_attribs=True) 
... class A: 
...     y: int 
...     x: Final[int] = 123                                                                                                                      

>>> A(y=1, x=2)                                                                                                                                  
A(y=1, x=2)

As you can see, x is unexpectedly an instance variable. If we're conforming to PEP 591, the correct behavior here would be to build a class equivalent to the following:

@attr.s(auto_attribs=True)
class A:
    x: ClassVar[int] = 123
    y: int

Environment:

  • CPython 3.8.8
  • attrs==20.3.0
  • OS: MacOS 11.2.3
@hynek
Copy link
Member

hynek commented Mar 24, 2021

I sounds like one line patch if Final is really equivalent to ClassVar. @euresti ? (I’m so sorry)

@dargueta
Copy link
Contributor Author

if Final is really equivalent to ClassVar

Ish. It is, only if there's a default value assigned.

@hynek
Copy link
Member

hynek commented Mar 27, 2021

Hmmm that complicates things of course.

@hynek hynek added the Typing Typing/stub/Mypy/PyRight related bugs. label Mar 27, 2021
@euresti
Copy link
Contributor

euresti commented Mar 27, 2021

Hmm. I think that PEP and attrs are in a little bit of a fight:

Type checkers should infer a final attribute that is initialized in a class body as being a class variable. Variables should not be annotated with both ClassVar and Final.

However to atts, the assignment is a default value not an actual assignment. So:

@attr.s(auto_attribs=True)
class A:
    x: Final[int] = 123

is a bit ambiguous. Attrs can't know if you mean the A.x is an instance variable that is Final or a class variable.

Also, looks like Python's dataclasses treat Final in the same way.

@euresti
Copy link
Contributor

euresti commented Mar 27, 2021

Ok looks like I said exactly what you said in the bug report. (Learn to read David)
Anyway what I was trying to say is that I'm not sure we can determine what the intent of the line is. Do you mean for x to be class or instance? If we make all Final with assignment class variables how would you create a Final instance value with a default value?

@dargueta
Copy link
Contributor Author

dargueta commented Mar 29, 2021

how would you create a Final instance value with a default value?

Use attr.ib(default=...)?

If we require that then we have to ask ourselves -- is that a breaking change? Part of me says yes because I bet you people out there are relying on that behavior and are unaware of that clause in PEP 591. (To be fair, the current behavior is logical.) At the same time, it's technically incorrect behavior and people are relying on a bug.

Also, looks like Python's dataclasses treat Final in the same way.

Oh dear...

@dargueta
Copy link
Contributor Author

Okay so after some thinking, considering that Python's native dataclasses have the same behavior as attrs, changing this would no longer make it a drop-in replacement and probably make a bunch of people angry. I'll file a ticket on bugs.python.org and see what they think there.

@burnpanck
Copy link

Hey, I couldn't find your ticket on bugs.pythonl.org, could you reference it here? It appears that in python 3.10 the behaviour of dataclass is indeed still like this, yet it disagrees with the documentation of Final. I did just run into this issue, and I would like to express my support to clarify that behaviour.

@MSK61
Copy link

MSK61 commented Dec 26, 2023

It depends on which you consider the source of truth: the behavior of python's dataclasses or what's explicitly spelled out in PEP 591. To me it looks like the latter is the formal specification, and I really don't care if following the specification in attrs would contradict the behavior in dataclasses.

As a workaround though, I managed to exclude Final class variables from being considered instance variables by dispensing with auto_attribs=True and marking the attributes explicitly, so something like this:

@attr.s
class A:
    x: Final[int] = 123
    y: int = attr.ib()

Obviously not the smartest solution, but it works.

@dargueta
Copy link
Contributor Author

dargueta commented Mar 8, 2024

@burnpanck Sorry, I completely forgot about this!

@dargueta
Copy link
Contributor Author

dargueta commented Mar 16, 2024

@burnpanck Looks like someone filed a ticket with Python a few months after I filed this ticket. The thread died but it seems like everyone's opposed to changing Python's behavior, and instead prefer modifying the PEP.

@burnpanck
Copy link

burnpanck commented Mar 17, 2024

Hey @dargueta, thanks for following up on this. In fact, that ticket has been migrated to GitHub, and it seems there is some recent movement there which may bring Final[ClassVar[...]] and permutations to Python 3.13. So that basically means we have a path for supporting final class variables in attrs as-well. Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing Typing/stub/Mypy/PyRight related bugs.
Projects
None yet
Development

No branches or pull requests

5 participants