- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 3k
 
          Fix Enum.value inference for Enums with @cached methods
          #19374
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| 
           Should I add something to the changelog? I think this is a user-facing change.  | 
    
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
| 
           We build changelog from the commit history, like #19138, so no need to add CHANGELOG entries in every PR:)  | 
    
        
          
                mypy/plugins/enums.py
              
                Outdated
          
        
      | node_types = ( | ||
| get_proper_type(n.type) if n else None | ||
| for n in stnodes | ||
| if n is None or not n.implicit | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this check is still needed - can there be any enum_members that are implicit? That should never happen AFAIC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like it is? With
diff --git i/mypy/plugins/enums.py w/mypy/plugins/enums.py
index 35338d091..c6e9f21b6 100644
--- i/mypy/plugins/enums.py
+++ w/mypy/plugins/enums.py
@@ -192,7 +192,7 @@ def enum_value_callback(ctx: mypy.plugin.AttributeContext) -> Type:
             node_types = (
                 get_proper_type(n.type) if n else None
                 for n in stnodes
-                if n is None or not n.implicit
+                if n is None
             )
             proper_types = [_infer_value_type_with_auto_fallback(ctx, t) for t in node_types]
             underlying_type = _first(proper_types)I get
$ pytest -k testValueFallbackWithCachedMethod
============================= test session starts ==============================
platform linux -- Python 3.12.6, pytest-8.3.5, pluggy-1.6.0
rootdir: /home/kevinmurphy/src/python/mypy
configfile: pyproject.toml
testpaths: mypy/test, mypyc/test
plugins: xdist-3.7.0, cov-6.1.1
6 workers [1 item]      
F                                                                        [100%]
=================================== FAILURES ===================================
______________________ testValueFallbackWithCachedMethod _______________________
[gw0] linux -- Python 3.12.6 /home/kevinmurphy/src/python/mypy/venv/bin/python3
data: /home/kevinmurphy/src/python/mypy/test-data/unit/check-enum.test:2543:
Failed: Unexpected type checker output (/home/kevinmurphy/src/python/mypy/test-data/unit/check-enum.test, line 2543)
----------------------------- Captured stderr call -----------------------------
Expected:
  main:19: note: Revealed type is "builtins.int"
  main:22: note: Revealed type is "builtins.int" (diff)
Actual:
  main:19: note: Revealed type is "builtins.int"
  main:22: note: Revealed type is "Any" (diff)
Alignment of first line difference:
  E: ...ote: Revealed type is "builtins.int"
  A: ...ote: Revealed type is "Any"
                               ^
Update the test output using --update-data (implies -n0; you can additionally use the -k selector to update only specific tests)
=========================== short test summary info ============================
FAILED mypy/test/testcheck.py::TypeCheckSuite::check-enum.test::testValueFallbackWithCachedMethod
============================== 1 failed in 2.26s ===============================There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I didn't mean to suggest only retaining None items:) I asked if the if clause can be removed altogether: n is None or not n.implicit should be true for all n coming from enum_members (all members should not be implicit - attrs with annotation and without value are assumed to be nonmembers according to the spec). Probably none of them should be None either, maybe just
node_types = (get_proper_type(info[name].type) for name in info.enum_members)
is enough now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, I think 1cedcf9 will work! I'll go ahead and squash that into the other commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. and resolve the upstream conflict while I'm at it.
1cedcf9    to
    e116ada      
    Compare
  
    Before this, adding an annotation like `@functools.cache` to any method on an `Enum` caused the inference for the class's `.value` to fail (i.e., become `Any`). Fixes python#19368 Co-authored-by: Stanislav Terliakov <[email protected]>
e116ada    to
    110a1a9      
    Compare
  
    | 
           According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅  | 
    
| 
           @sterliakov bump?  | 
    
    
      
        1 similar comment
      
    
  
    | 
           @sterliakov bump?  | 
    
Before this, adding an annotation like
@functools.cacheto any method on anEnumcaused the inference for the class's.valueto fail (i.e., becomeAny).Fixes #19368