Clarify meaning of Final in dataclasses

[GBeauregard]

BPO	45384
Nosy	@ericvsmith, @carljm, @GBeauregard, @saaketp

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-10-05.23:00:02.707>
labels = ['type-bug', 'library']
title = 'Accept Final as indicating ClassVar for dataclass'
updated_at = <Date 2022-01-21.01:21:16.689>
user = 'https://github.com/GBeauregard'

bugs.python.org fields:

activity = <Date 2022-01-21.01:21:16.689>
actor = 'med2277'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-10-05.23:00:02.707>
creator = 'GBeauregard'
dependencies = []
files = []
hgrepos = []
issue_num = 45384
keywords = []
message_count = 12.0
messages = ['403277', '403552', '403553', '403555', '403560', '403565', '403615', '403616', '403617', '403890', '403931', '411067']
nosy_count = 6.0
nosy_names = ['eric.smith', 'carljm', 'GBeauregard', 'saaketp', 'msully4321', 'med2277']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue45384'
versions = []

Linked PRs

• gh-89547: Add support for nesting special forms like Final #116092
• gh-89547: Support for nesting special forms like Final #116096
• gh-89547: typing.rst: Add note about change in behavior with ClassVar/Final #116686

[GBeauregard]

PEP-591 for the Final Attribute states "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."

This is a bit of a typing conflict for dataclasses, where ClassVar is used to indicate a desired library behavior, but one may want to indicate Final.

I propose accepting the Final attribute as an indicator of a ClassVar in dataclasses class bodies in order to be better compatible with the Final PEP.

There is at least one edge case that would need to be handled where someone might want to explicitly mark a dataclass field Final, which could be allowed as a field:
a: Final[int] = dataclasses.field(init=False, default=10)

[GBeauregard]

Hi Eric,

I've been shopping this idea around on the mailing list and haven't received any objections. Do you have any concerns? Can we handle Final with the same checks as ClassVar?

Regarding potentially merging a change, I'm not sure where this falls between a bug fix and a feature. On one hand the PEP-591 instruction to typecheckers on how to treat Final is relatively absolute and the dataclasses behavior could be considered a buggy interaction with it. On the other hand this is sorta a new behavior. Do you have any opinions? Should I not worry about it when working on patch and let core devs figure out if it would need backported?

I've read through the dataclasses code and I think I can implement this myself and submit a PR, but I may need a bit of a heavy handed code review since I've only recently started getting serious with Python.

Thanks for your time and work on dataclassess.

[ericvsmith]

I was waiting for someone smarter than me to chime in on one of the discussions.

I wouldn't worry about whether it's a bug or feature, at this point. Assuming buy-in from type checkers, I'd probably call it a bug, but I can be reasoned with.

One thing I don't understand is what you mean by:

"""
There is at least one edge case that would need to be handled where someone might want to explicitly mark a dataclass field Final, which could be allowed as a field:
a: Final[int] = dataclasses.field(init=False, default=10)
"""

I assume we'd want to treat it like a ClassVar, whatever that does. What's your thought? Are you saying ClassVar works incorrectly in this instance.

[GBeauregard]

When I originally submitted the issue I hadn't finished going through all of the dataclasses code and it hadn't even occurred to me that it could be valid to use ClassVar with field(). I (wrongly) assumed this would always raise and that field() is only valid for things intended to be instance vars.

I do find this behavior a little surprising, but on reflection I don't think it's explicitly wrong as long we raise for default_factory like it currently does. I think it's then appropriate to just do the exact same behavior for Final as ClassVar.

I'm going to start working on a PR, thanks for your feedback.

[saaketp]

Treating Final as ClassVar by default may be fine,
but it should not throw when using default_factory like ClassVar does.

There are valid uses of Final with instance variable when one would want the value to be unchanged after the __init__ runs
but different instances can be initialized with different values that are generated by a default_factory.

A quick search on github for this pattern gives this
https://github.com/166MMX/hiro-python-library/blob/fb29e3247a8fe1b0f7dc4e68141cf7340a8dd0a5/src/arago/hiro/model/ws.py#L120
which will break if Final throws when using default_factory.

PEP-591 says:
Type checkers should infer a final attribute _that is initialized in a class body_ as being a class variable.
When using default_factory the attribute is not initialized inside the class body but when the instance is initialized.
So allowing instance level Final with default_factory will not be going against the PEP.

[GBeauregard]

Yeah, I was just discussing this with someone in IRC, and I appreciate an example of it in the wild.

I agree there's some wiggle room with what "initialized in a class body" means when it comes to dataclasses.

I see several interpretations there, and I would strongly prefer feedback from typing folks, particularly since they would be responsible for implementing any Final default_factory exceptions.

On the implementation side this does complicate things a bit depending on specifics. Are Final default_factory fields real fields or pseudo-fields? (i.e. are they returned by dataclasses.fields()?) Depending on how this works dataclasses might need a bit more refactoring than I'd be the right person for, but I'm still willing to give it a shot.

[carljm]

Are Final default_factory fields real fields or pseudo-fields? (i.e. are they returned by dataclasses.fields()?)

They are real fields, returned by dataclasses.fields().

In my opinion, the behavior change proposed in this bug is a bad idea all around, and should not be made, and the inconsistency with PEP-591 should rather be resolved by explicitly specifying the interaction with dataclasses in a modification to the PEP.

Currently the meaning of:

@dataclass
class C:
    x: Final[int] = 3

is well-defined, intuitive, and implemented consistently both in the runtime and in type checkers. It specifies a dataclass field of type int, with a default value of 3 for new instances, which can be overridden with an init arg, but cannot be modified (per type checker; runtime doesn't enforce Final) after the instance is initialized.

Changing the meaning of the above code to be "a dataclass with no fields, but one final class attribute of value 3" is a backwards-incompatible change to a less useful and less intuitive behavior.

I argue the current behavior is intuitive because in general the type annotation on a dataclass attribute applies to the eventual instance attribute, not to the immediate RHS -- this is made very clear by the fact that typecheckers happily accept x: int = dataclasses.field(...) which in a non-dataclass context would be a type error. Therefore the Final should similarly be taken to apply to the eventual instance attribute, not to the immediate assignment. And therefore it should not (in the case of dataclasses) imply ClassVar.

I realize that this means that if we want to allow final class attributes on dataclasses, it would require wrapping an explicit ClassVar around Final, which violates the current text of PEP-591. I would suggest this is simply because that PEP did not consider the specific case of dataclasses, and the PEP should be amended to carve out dataclasses specifically.