Skip to content
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

NameError on Enum #402

Open
2 tasks done
jolaf opened this issue Sep 11, 2023 · 19 comments
Open
2 tasks done

NameError on Enum #402

jolaf opened this issue Sep 11, 2023 · 19 comments
Labels

Comments

@jolaf
Copy link

jolaf commented Sep 11, 2023

Things to check first

  • I have searched the existing issues and didn't find my bug already reported there

  • I have checked that my bug is still present in the latest release

Typeguard version

4.1.5

Python version

3.10.12

What happened?

Traceback (most recent call last):
  File "test.py", line 3, in <module>
    from A import A
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 688, in _load_unlocked
  File "/home/jolaf/.local/lib/python3.10/site-packages/typeguard/_importhook.py", line 98, in exec_module
    super().exec_module(module)
  File "A.py", line 7, in <module>
    A()
  File "A.py", line 5, in __init__
    def __init__(self, e: E = E.X) -> None:
NameError: name 'E' is not defined. Did you mean: 'e'?

How can we reproduce the bug?

test.py:

from typeguard import install_import_hook
with install_import_hook(('A',)):
    import A

A.py:

from enum import Enum
class A:
    class E(Enum):
        X = 1
    def __init__(self, e: E = E.X) -> None:
        print("OK")
A()
$ python3 A.py
OK

$ python3 test.py
<crash>
@jolaf jolaf added the bug label Sep 11, 2023
@agronholm
Copy link
Owner

Have you tried e: A.E?

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

No I didn't, but as is it's ok with mypy.

@agronholm
Copy link
Owner

Mypy doesn't have to look up things at run time.

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

That's true, but how do we decide whether typeguard should understand a particular annotation or not? My idea is we check whether mypy understands it, and if yes, than typeguard should too, if possible.

I don't think we should expect a user to change their code for typeguard to understand it.

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

And if for some reason typeguard can't process some annotation correctly, at least it should not crash, preventing the app from running further.

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

Have you tried e: A.E?

Now I have:

  File "A.py", line 5, in A
    def __init__(self, e: A.E = A.E.X) -> None:
NameError: name 'A' is not defined

@agronholm
Copy link
Owner

That should have been: def __init__(self, e: A.E = E.X) -> None:
The reason for this is that the default gets assigned during the definition of the method, at which time A does not exist but E does! When that method is called, however, it's the opposite: A exists in the scope but E does not.

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

That sounds extremely weird, and doesn't follow any guidelines on writing annotations and otherwise referencing variables.
So I wouldn't expect someone to describe a method argument like this. :(

Also, it doesn't help:

  File "A.py", line 5, in A
    def __init__(self, e: A.E = E.X) -> None:
NameError: name 'A' is not defined

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

It seems that mypy considers both E and A.E to be correct.
So I think typeguard should ideally consider both of them to be correct.
Or, if that's not possible, just skip the respective checks or issue a warning.

@agronholm
Copy link
Owner

It's not like Typeguard is considering E to be incorrect, it just doesn't have access to E in the scope of the call to __init__(). The only solution I can think of is doing what I did with __new__() in the other issue: aliasing E = A.E.

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

Well, that sounds reasonable, isn't it?

@agronholm
Copy link
Owner

I just have a creeping feeling that I'm adding hacks on top of hacks and it's going to lead to more regressions down the line. But what else can I do? 🤷

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

Yeah, I understand that. :(

It seems like the only other thing possible is detecting situations like this and just skipping checking for them.
But hacks are probably better.

@agronholm
Copy link
Owner

By the way, that example works if you add from __future__ import annotations or add quotes around A.E.

@agronholm
Copy link
Owner

I have a failing unit test for this now.

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

Yep, but it works very weirdly:

$ python3 A.py 
OK

$ python3 test.py 
OK
OK

Why the extra instantiation?

@agronholm
Copy link
Owner

Does typeconfig.config.debug_instrumentation = True give any insight to that?

@agronholm
Copy link
Owner

You're instantiating A() both in test.py and A.py so this shouldn't be surprising.

@jolaf
Copy link
Author

jolaf commented Sep 11, 2023

Oh, my bad. Instantiation is definitely not needed in test.py. Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants