[ty] support enum _value_ annotation#22228
Open
silamon wants to merge 12 commits intoastral-sh:mainfrom
Open
Conversation
silamon
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
carljm
changed the title
_value_ annotation_value_ annotation
Typing conformance results improved 🎉The percentage of diagnostics emitted that were expected errors increased from 84.96% to 84.99%. The percentage of expected errors that received a diagnostic increased from 75.14% to 75.25%. Summary
True positives addedDetails
Optional Diagnostics RemovedDetails
Optional Diagnostics AddedDetails
|
|
Contributor
Author
|
Aside from some failing CI, there's still 2 more issues that got identified: from enum import Enum
class Color(Enum):
_value_: int
RED = 1
reveal_type(Color.RED.value) # revealed: Literal[1]
reveal_type(Color.RED._value_) # revealed: Literal[1]The Second, this example came out the comformance testing: from enum import Enum
class Planet2(Enum):
_value_: str
def __init__(self, value: int, mass: float, radius: float):
self._value_ = value # E
MERCURY = (1, 3.303e23, 2.4397e6)It's throwing an diagnostic currently on the last MERCURY line, while it shouldn't. While it's obviously what's happening here, it's not so obviously to me how to fix it. |
Member
|
Thank you for working on this. Almost the entire team is out this week. It may take a few days before someone finds time to answer your question. We're sorry for that. Happy holidays. |
AlexWaygood
reviewed
Jan 16, 2026
Member
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
Nice, thank you! Sorry for the delay in reviewing this!!
Member
I think inferring
The spec explains what to do here at https://typing.python.org/en/latest/spec/enums.html#member-values |
silamon
force-pushed
the
enum_value_annotation
branch
3 times, most recently
from
13c83bc to
8e2b01e
Compare
Contributor
Author
|
I've applied your patch (thanks for that), and worked further on it. It's a better place than what I had first. Conformance testing shows what I expected, unit tests succeed including the stub example. |
carljm
approved these changes
Jan 23, 2026
Contributor
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
Thank you, this looks good!
I'll await review and then will include fixes for the markdown linting
For future reference: this should be a trivial fix with uvx prek -a, so it probably makes more sense to just go ahead and fix it rather than waiting for review first?
AlexWaygood
reviewed
Jan 23, 2026
Comment on lines
79
to
84
| /// Extracts the expected enum member type from `__init__` method parameters. | ||
| fn extract_init_member_type( | ||
| db: &'db dyn Db, | ||
| _class: StaticClassLiteral<'db>, | ||
| scope: ScopeId<'db>, | ||
| ) -> Option<Type<'db>> { |
Member
There was a problem hiding this comment.
I don't think we can assume that the first parameter of an Enum class's __init__ method always provides the _value_ type. I think we need to look at the type assigned to self._value_ inside the enum's __init__ method rather than attempting to extract the annotated type of the first parameter of the __init__ method
Contributor
There was a problem hiding this comment.
As I understand it, it's the entire signature of __init__ that determines the expected type assigned to members. If __init__ takes multiple arguments, it's a tuple of those types that we expect. If __init__ takes only a single argument, it's that type that is expected. That's what's implemented here, and it looks correct to me.
If we are discussing the type ultimately revealed by accessing _value_ on a member, that may be something else? But it also doesn't seem to be touched by this PR.
Member
There was a problem hiding this comment.
Oh I see, sorry. Yes, looks like you're right.
Contributor
There was a problem hiding this comment.
I think we need to look at the type assigned to
self._value_inside the enum's__init__method
I don't think other type checkers do this. Pyright appears to just fallback to Any as the type of the _value_ member, if there is an __init__.
So I do think that's an improvement we need (either the pyright fallback, or the more complex analysis you suggest), and it should probably go in this PR, although technically I think this PR is a strict improvement either way. Here's a test case:
from enum import Enum
class MyEnum(Enum):
def __init__(self, x: int, y: str):
self._value_ = y
RED = (1, "red")
BLUE = (2, "blue")
reveal_type(MyEnum.RED._value_)Pyright reveals Any here (but reveals the precise assigned type if there is no __init__). We currently (and in this PR) reveal tuple[Literal[1], Literal["red"]], which is wrong. The ideal thing to reveal would be Literal["red"], but I think that would be quite hard and not worth it. Revealing str instead of Any doesn't seem too hard (though I'm still not sure it's worth it, given that there will always be more complex cases where we have to fall back to a union, if there are multiple assignments to _value_ in __init__, and that union could cause false positives.)
Member
There was a problem hiding this comment.
Yeah, I agree it's out of scope for this PR, which as you say is a strict improvement over the status quo. Sorry for getting confused!
If the enum has a custom __init__ method I think we would ideally fallback to the _value_ annotation (either in __init__ or in the class body), if it's available, and fallback to Any if not. But if that's hard I'd also be okay with only falling back to _value_ if it's provided in the class body and ignoring assignments in __init__.
Member
There was a problem hiding this comment.
and to be clear, we should always make sure .value is consistent with ._value_, so this has implications for the type we store in the EnumMetadata::members mapping from enum members to their values.
Contributor
There was a problem hiding this comment.
I think relatedly, we should probably revisit the earlier decision in this PR that it's OK for _value_ to reveal Literal[1] in this example, rather than int:
from enum import Enum
class MyEnum(Enum):
_value_: int
RED = 1
BLUE = 2
reveal_type(MyEnum.RED._value_)The reason is that it's possible for the RHS of these assignments to be e.g. a call that returns Any or Unknown, and in that case we should really reveal int, not Any or Unknown. In a sense this is just another version of astral-sh/ty#136, where it does matter that we respect the explicit type annotation given by the user.
Contributor
There was a problem hiding this comment.
(Sorry, wasn't getting live updates on the PR thread, and posted all my comments here, and the main request-changes comment, before I saw any of your comments. I kind of changed my mind about these issues being out of scope for this PR, because I think the need to connect annotated _value_ type to eventual inferred type of _value_ / value member attributes could significantly change the implementation here.)
Member
There was a problem hiding this comment.
it makes me a bit sad that we'll lose literal types for .value for things like this -- Literal types seem much more important for enum values than for most other areas of Python typing:
class Foo(Enum):
_value_: str
X = 'A'
Y = 'B'but I do agree that it's important we don't infer Any or Unknown for .value for things like this:
def returns_unknown():
return "foo"
class Bar(Enum):
_value_: str
X = returns_unknown()
Y = returns_unknown()Since enums are heavily special-cased anyway, it's possible we could do some kind of syntactic check here to see whether the type was inferred due to a literal value on the right-hand side (which will be by far the most common case) or something else, like a function call.
But given that _value_ annotations are pretty rare anyway, maybe we shouldn't worry about this too much. I definitely don't think we should make a rule that says we prefer the inferred type only if the inferred type is fully static; that would be very inconsistent with what e.g. mypy does, what users expect, and what I believe we're currently planning to do to address astral-sh/ty#136 (though the title of that issue still mentions only preferring "more precise" declared types).
carljm
requested changes
Jan 23, 2026
Contributor
There was a problem hiding this comment.
Given the discussion in the sub-thread, I think we should update this PR so that a) we always respect explicit _value_ annotation, if there is one, when inferring a type for accessing _value_ or value attribute on an enum member, and b) we fall back to Any for our inference of _value_ / value attributes if the enum has an __init__ method. (I think that if we want to do anything more precise than Any here -- and I'm not sure we should -- that's subtle enough that it should be a separate PR.)
I do also think it's worth considering just merging EnumClassInfo into EnumMetadata. I don't think this is critical, but it's also not clear to me that there's any benefit to them being separate. (And given the above changes will require having value_sunder_type also influence our inference of _value_ / value attribute types, it may simplify that implementation anyway.)
Contributor
Author
|
I'm going to work further on this later, thank you for your feedback. |
silamon
force-pushed
the
enum_value_annotation
branch
from
cebc4ae to
39a1c81
Compare
Contributor
Author
|
I've did the remarks:
Somehow along the way I started to introduce something that resolves many types to Unknown, which causes a lot of tests to fail. I haven't been able to find it yet, if somebody could help me out I would appreciate it. |
Contributor
|
@charliermarsh this might be another one in the queue that you could take a look at what it needs to get land-ready? |
charliermarsh
force-pushed
the
enum_value_annotation
branch
from
39a1c81 to
f5c1758
Compare
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
sphinx
trio
flake8
|
Member
|
(Attempting to push a change to get the remaining conformance tests passing.) |
charliermarsh
force-pushed
the
enum_value_annotation
branch
from
ba2eea8 to
3179672
Compare
charliermarsh
approved these changes
Feb 15, 2026
Member
|
I will let @carljm merge (or not) since he is more well-versed in the semantics. |
AlexWaygood
reviewed
Feb 16, 2026
Member
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
I understand the rationale for resolving .value and ._value_ to Any if __init__ is defined. But I don't understand the rationale for not doing any validation of the right-hand side of member assignments if __init__ is assigned. It seems like we could fairly easily validate the right-hand side of member assignments against the parameters that __init__ expects.
Is this just a short-term simplification? If so, that's fine, but we should add some TODO comments and make clear in the tests that we could add validation against __init__ parameters in the future. (But I also don't think it would be too complex to just do it in this PR.)
| self._value_ = value # error: [invalid-assignment] | ||
|
|
||
| MERCURY = (1, 3.303e23, 2.4397e6) | ||
| SATURN = "saturn" |
Member
There was a problem hiding this comment.
I don't understand the rationale for "falling back to Any" for member validation here. This fails at runtime -- could we not catch the runtime error by synthesizing a call to the __init__ signature if __init__ is defined? (The right-hand side of the SATURN assignment should be an int, float, float tuple, because those are the parameters expected by __init__ here.)
Member
There was a problem hiding this comment.
Hmm yeah, this seems wrong. I think I was confused by this conformance test:
class Planet2(Enum):
_value_: str
def __init__(self, value: int, mass: float, radius: float):
self._value_ = value # E
MERCURY = (1, 3.303e23, 2.4397e6)
Contributor
There was a problem hiding this comment.
Yeah there should be an error on this line, we should be validating against the __init__ signature. I think the previous version of this PR did that correctly?
Comment on lines
159
to
175
| When `__init__` is defined but no explicit `_value_` annotation exists, we also fall back to `Any` | ||
| for member value validation: | ||
|
|
||
| ```py | ||
| from enum import Enum | ||
|
|
||
| class Planet2(Enum): | ||
| def __init__(self, mass: float, radius: float): | ||
| self.mass = mass | ||
| self.radius = radius | ||
|
|
||
| MERCURY = (3.303e23, 2.4397e6) | ||
| VENUS = (4.869e24, 6.0518e6) | ||
|
|
||
| reveal_type(Planet2.MERCURY.value) # revealed: Any | ||
| reveal_type(Planet2.MERCURY._value_) # revealed: Any | ||
| ``` |
Member
There was a problem hiding this comment.
I'm confused: the prose here describes "falling back to Any for member validation". But that's not what the test demonstrates. The test demonstrates that we infer Any for the .value and ._value_ attributes. But when it comes to validation of the right-hand sides of the MERCURY and VENUS assignments, this test uses right-hand sides which are compatible with the __init__ signature in both cases, so the test would still pass even if we did not "fallback to Any for member validation".
rayzeller
added a commit
to rayzeller/ruff
that referenced
this pull request
Feb 20, 2026
…pport Strip the `_value_` annotation lookup logic (`enum_value_annotation_type` and `own_value_annotation`) to avoid overlap with astral-sh#22228, which handles `_value_` semantics more broadly. Custom `__new__` enums now always fall back to `Any` for the member value type. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rayzeller
added a commit
to rayzeller/ruff
that referenced
this pull request
Feb 20, 2026
…pport Strip the `_value_` annotation lookup logic (`enum_value_annotation_type` and `own_value_annotation`) to avoid overlap with astral-sh#22228, which handles `_value_` semantics more broadly. Custom `__new__` enums now always fall back to `Any` for the member value type. Also remove the redundant known-class skip list from `has_custom_enum_new` — the vendored path check already covers all stdlib classes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rayzeller
added a commit
to rayzeller/ruff
that referenced
this pull request
Feb 20, 2026
…new` Remove the `_value_` annotation lookup logic to avoid overlap with astral-sh#22228, which handles `_value_` semantics more broadly. Custom `__new__` enums now always fall back to `Any`. Also remove the redundant known-class skip list from `has_custom_enum_new` — the vendored path check already covers all stdlib classes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
carljm
requested changes
Feb 21, 2026
Contributor
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
Just reviewed the tests for now -- will review the code once it looks like the tests are specifying the right semantics.
It looks to me like @AlexWaygood 's comments from a few days ago were exactly right -- no need to wait for me to confirm!
Comment on lines
+117
to
+118
| # In stub files, `[]` is not exempt from type checking (only `...` is). | ||
| PURPLE = [] # error: [invalid-assignment] |
Contributor
There was a problem hiding this comment.
Kind of an odd comment and test. Not necessarily opposed to including the test, but I don't really get why anyone would think that [] would be a special case here.
| self._value_ = value # error: [invalid-assignment] | ||
|
|
||
| MERCURY = (1, 3.303e23, 2.4397e6) | ||
| SATURN = "saturn" |
Contributor
There was a problem hiding this comment.
Yeah there should be an error on this line, we should be validating against the __init__ signature. I think the previous version of this PR did that correctly?
| ### `_value_` annotation with `__init__` | ||
|
|
||
| When `__init__` is defined, member values are passed through `__init__` rather than directly | ||
| assigned to `_value_`, so we fall back to `Any` for member value validation. The `_value_` |
Contributor
There was a problem hiding this comment.
We shouldn't fall back to Any for validation. We should validate against the __init__ signature.
We should fall back to Any for the type of .value or ._value_ on individual members, if there is no annotated _value_ type on the enum class. (If there is, we should use that -- and validate assignments to self._value_ in __init__ according to that annotation, as you already do.)
Comment on lines
159
to
160
| When `__init__` is defined but no explicit `_value_` annotation exists, we also fall back to `Any` | ||
| for member value validation: |
Contributor
There was a problem hiding this comment.
We should never "fall back to Any for member value validation." What this test (correctly) shows is that we are falling back to Any for the type of .value and ._value_ on the resulting members. But this comment is wrong.
Comment on lines
177
to
192
| ### Inherited `_value_` annotation | ||
|
|
||
| A `_value_` annotation on a parent enum is not inherited by subclasses for the purpose of member | ||
| value validation: | ||
|
|
||
| ```py | ||
| from enum import Enum | ||
|
|
||
| class Base(Enum): | ||
| _value_: int | ||
|
|
||
| class Child(Base): | ||
| A = 1 | ||
| B = "not checked against int" | ||
|
|
||
| reveal_type(Child.A.value) # revealed: Literal[1] |
Contributor
There was a problem hiding this comment.
Is this specified, or just something observed in the ecosystem?
Member
There was a problem hiding this comment.
I think it's unspecified... I think Pyright does inherit _value_, mypy does not.
Contributor
There was a problem hiding this comment.
I think this PR currently doesn't inherit __init__ from a base class, and it needs to do so in order to match runtime behavior on e.g. this example:
from enum import Enum
class Base(Enum):
def __init__(self, a: int, b: str):
self._value_ = a
class Child(Base):
A = (1, "foo")
B = "should be checked against __init__" # should error, currently doesn't
reveal_type(Child.A.value) # should reveal `Any`, currently reveals `tuple[Literal[1], Literal["foo"]]`At runtime A and B do pass through Base.__init__, so we should also model that.
And once we model that correctly, I think the only consistent/correct approach is also to inherit _value_ annotation from a base.
charliermarsh
force-pushed
the
enum_value_annotation
branch
from
3179672 to
3a7d627
Compare
charliermarsh
force-pushed
the
enum_value_annotation
branch
from
3a7d627 to
6ddf331
Compare
Member
|
(We realized offline that I had addressed some feedback but pushed to the wrong branch... So I just updated this PR now.) |
carljm
requested changes
Feb 23, 2026
Comment on lines
177
to
192
| ### Inherited `_value_` annotation | ||
|
|
||
| A `_value_` annotation on a parent enum is not inherited by subclasses for the purpose of member | ||
| value validation: | ||
|
|
||
| ```py | ||
| from enum import Enum | ||
|
|
||
| class Base(Enum): | ||
| _value_: int | ||
|
|
||
| class Child(Base): | ||
| A = 1 | ||
| B = "not checked against int" | ||
|
|
||
| reveal_type(Child.A.value) # revealed: Literal[1] |
Contributor
There was a problem hiding this comment.
I think this PR currently doesn't inherit __init__ from a base class, and it needs to do so in order to match runtime behavior on e.g. this example:
from enum import Enum
class Base(Enum):
def __init__(self, a: int, b: str):
self._value_ = a
class Child(Base):
A = (1, "foo")
B = "should be checked against __init__" # should error, currently doesn't
reveal_type(Child.A.value) # should reveal `Any`, currently reveals `tuple[Literal[1], Literal["foo"]]`At runtime A and B do pass through Base.__init__, so we should also model that.
And once we model that correctly, I think the only consistent/correct approach is also to inherit _value_ annotation from a base.
| _value_: str | ||
|
|
||
| def __init__(self, value: int, mass: float, radius: float): | ||
| self._value_ = value # error: [invalid-assignment] |
Contributor
There was a problem hiding this comment.
Let's also have a happy-path test with both _value_ annotation and __init__, where the assignment to _value_ in __init__ is correct.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Summary
The
_value_attribute is used inside an Enum class to explicitly define the underlying value of an enum member. Typing can be verified on the members.Test Plan
Added a mdtest