gh-119581: Add a test of InitVar with name shadowing #119582
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As originally discussed in python/mypy#17219, MyPy has had a false-positive bug report because it errors when a dataclass has methods that shadow an `InitVar` field. It is actually a bit surprising that this works, it turns out that `__annotations__` "remembers" field assignments even if the bound names are later overwritten by methods; it will *not* work to provide a default value. There have been multiple bug reports on MyPy so we know people are actually relying on this in practice; most likely it comes up when a dataclass wants to take a "raw" value as an InitVar and transform it somehow in `__post_init__` into a different value before assigning it to a field; in that case they may choose to make the actual field private and provide a property for access. I currently provide a test of the happy path where there is no default value provided, but no tests of the behavior when no default is provided (in which case the property will override the default) and no documentation (because I'm not sure we want to consider this behavior officially supported). The main goal is to have a regression test since it would be easy for a refactor to break this.
AlexWaygood
reviewed
May 28, 2024
There was a problem hiding this comment.
Thanks! It seems likely to me that this probably only worked "by accident" rather than being something that was deliberately designed. But nonetheless, it seems like useful behaviour that's worth preserving; and as you say, it seems like lots of people are relying on it now. So, I agree that this is a useful test to add.
(Whether or not type checkers support this pattern is irrelevant to whether it should be supported at runtime: lots of people use dataclasses without running a type checker on their code.)
Co-authored-by: Alex Waygood <[email protected]>
AlexWaygood
approved these changes
May 28, 2024
AlexWaygood
added
needs backport to 3.12
ericvsmith
approved these changes
May 28, 2024
|
Thanks @stroxler for the PR, and @AlexWaygood for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
miss-islington
pushed a commit
to miss-islington/cpython
that referenced
this pull request
May 28, 2024
…119582) (cherry picked from commit 6ec3712) Co-authored-by: Steven Troxler <[email protected]>
miss-islington
pushed a commit
to miss-islington/cpython
that referenced
this pull request
May 28, 2024
…119582) (cherry picked from commit 6ec3712) Co-authored-by: Steven Troxler <[email protected]>
|
GH-119672 is a backport of this pull request to the 3.13 branch. |
|
GH-119673 is a backport of this pull request to the 3.12 branch. |
AlexWaygood
pushed a commit
that referenced
this pull request
May 28, 2024
AlexWaygood
pushed a commit
that referenced
this pull request
May 28, 2024
estyxx
pushed a commit
to estyxx/cpython
that referenced
this pull request
Jul 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As originally discussed in python/mypy#17219,
MyPy has had a false-positive bug report because it errors when a dataclass has methods that shadow an
InitVarfield.It is actually a bit surprising that this works, it turns out that
__annotations__"remembers" field assignments even if the bound names are later overwritten by methods; it will not work to provide a default value.There have been multiple bug reports on MyPy so we know people are actually relying on this in practice; most likely it comes up when a dataclass wants to take a "raw" value as an InitVar and transform it somehow in
__post_init__into a different value before assigning it to a field; in that case they may choose to make the actual field private and provide a property for access.I currently provide a test of the happy path where there is no default value provided, but no tests of the behavior when no default is provided (in which case the property will override the default) and no documentation (because I'm not sure we want to consider this behavior officially supported).
The main goal is to have a regression test since it would be easy for a refactor to break this.
dataclassesunit tests should record behavior of shadowed init vars #119581