Clarify meaning of Final in dataclasses

@ericvsmith

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

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)

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.

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.

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.

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.

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.

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.

Sounds great, feel free to open a PR and I'll take a look. (I haven't checked recently but I think there's a place in typing._type_check or similar where Final/ClassVar are hardcoded; that's probably what needs to be changed.)

@hmc-cs-mdrissi

Thanks for taking on the runtime change, @hmc-cs-mdrissi ! I can do the typing spec PR: https://discuss.python.org/t/treatment-of-final-attributes-in-dataclass-likes/47154/4

I've gotten,

>>> class a:
...     x: ClassVar[Final[int]]
>>> class a:
...     x: Final[ClassVar[int]]

both to work. But a different test case fails that expected ClassVar[ClassVar[int]] to fail. My preference is while that's weird and not meaningful allowing some special forms in ClassVar like Final but not others (itself) will likely add unneeded runtime validation complexity. So I'd lean to allow ClassVar[ClassVar[int]] at runtime while type checkers are free to forbid it.

Should Annotated also be allowed to wrap either Final or ClassVar, and/or be wrapped by them?
The question of Annotated wrapping ClassVar/Final is really a separate one, but similar so it may make sense to handle them together.

I'm also leaning to include test cases for Annotated interaction and ensure that

class ACF:
  x: Annotated[ClassVar[Final[int]], "a decoration"]

and similar variations are well tested.

I think it's OK to allow ClassVar[ClassVar[int]]. It's weird but not especially harmful.

Type checkers in general should allow Annotated at any level around a qualifier, so I agree that your example should be allowed.

gh-89547: Support for nesting special forms like Final (#116096)

gh-89547: typing.rst: Add note about change in behavior with ClassVar…

pythongh-89547: typing.rst: Add note about change in behavior with Cl…

pythongh-89547: Support for nesting special forms like Final (python#…

pythongh-89547: typing.rst: Add note about change in behavior with Cl…

@carljm

Coming from the runtime type-checking side, now that #116096 is merged, I noticed that dataclass will handle ClassVar[Final[T]] correctly, since it already checks for ClassVar, but unwrapping special forms for inspecting if is InitVar or ClassVar is still TBD. (ie Annotated[ClassVar/InitVar, ...], Final[ClassVar[T]])
Just checking, this (hypothetical) dataclasses PR is still waiting on @carljm 's typing spec & subsequent approval ?
Or is the typing consensus we need outside of lifting the runtime limitations on the dataclasses implementation?

I've submitted a pull request to the typing spec: python/typing#1669

The typing spec applies to typecheckers, not to the runtime, so I don't think a dataclasses behavior change to unwrap type qualifiers looking for ClassVar necessarily needs to wait for approval of the typing spec change. That said, if the typing council chooses to specify that Final always implies ClassVar, even in dataclasses (the opposite of what my PR proposes), that would suggest a very different change to the runtime behavior (where dataclasses would start to detect Final and treat it as ClassVar), so it may be best to wait for a typing council decision.

pythongh-89547: Support for nesting special forms like Final (python#…

pythongh-89547: typing.rst: Add note about change in behavior with Cl…

Navigation Menu

Repository navigation

Clarify meaning of Final in dataclasses #89547

Linked PRs

14 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Clarify meaning of Final in dataclasses #89547

Description

Linked PRs

Activity

GBeauregard commented on Oct 5, 2021

GBeauregard commented on Oct 9, 2021

ericvsmith commented on Oct 9, 2021

GBeauregard commented on Oct 9, 2021

saaketp commented on Oct 10, 2021

GBeauregard commented on Oct 10, 2021

carljm commented on Oct 11, 2021

14 remaining items

JelleZijlstra commented on Feb 28, 2024

carljm commented on Feb 29, 2024

hmc-cs-mdrissi commented on Feb 29, 2024

JelleZijlstra commented on Feb 29, 2024

JWCS commented on Mar 27, 2024

carljm commented on Mar 29, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions