gh-133956 fix bug where dataclass wouldn't detect ClassVar fields if ClassVar was re-exported from a module other than typing#140541
Conversation
5ecae28 to
2f64527
Compare
@JelleZijlstra : In May, you said that you would be happy to review a PR which contains the fix and has tests. I believe that this PR meets those criteria. Would you review it? |
|
@JelleZijlstra are there any updates on this? |
|
@taminomara : You might also want to try pinging people in the issue thread too. They might ignore PRs if they do not have enough context. Thank you for making this patch. I briefly glanced at it when you first made it a couple months ago and it looked like what I had in mind. Happy holidays! |
d293967 to
91d0232
Compare
|
Rebased onto current main, no functional changes. Pre-rebase changeset was 8a855b4, saved in gh-133956-broken-classvar-pre-rebase. |
This PR continues work by @dzherb from #134073.
I've simplified the code and removed strict checks from
_is_type. Supporting arbitrary levels of module nesting turned out to be beneficial for overall code complexity, so I've included it to this PR; I can send it as a separate PR with a separate issue, though.Motivation for current implementation
I think we shouldn't assume anything about user-provided type annotations. All we know from annotation
ty.ClassVaris that there is an objectty, and we're getting its attributeClassVar. We shouldn't assume thattyis a module, nor that it doesn't implement some custom__getattr__logic; instead, we should just do what the annotation tells us to do.dataclasses: Synthetic__init__method on dataclass has brokenClassVarannotation under some conditions. #133956