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

stubtest: __all__ check seems to ignore *.allowlist files #17277

Closed
ncoghlan opened this issue May 22, 2024 · 2 comments
Closed

stubtest: __all__ check seems to ignore *.allowlist files #17277

ncoghlan opened this issue May 22, 2024 · 2 comments
Labels
bug mypy got something wrong

Comments

@ncoghlan
Copy link

ncoghlan commented May 22, 2024

Bug Report

mypy stubtest started failing on contextlib2 a while back, and I'm finally getting around to trying to fix it. Most of the issues have been resolved just by updating the stub to the latest version from typeshed (with the appropriate edits for contextlib2), but I had trouble getting this error to go away:

error: contextlib2.__all__ names exported from the stub do not correspond to the names exported at runtime. This is probably due to things being missing from the stub or an inaccurate `__all__` in the stub
Stub: in file /home/ncoghlan/devel/contextlib2/contextlib2/__init__.pyi
Names exported in the stub but not at runtime: []
Runtime:
Names exported at runtime but not in the stub: ['ContextStack']

For the specific case of contextlib2, I was able to resolve it by just dropping ContextStack from the runtime __all__ (it's a long-deprecated API that nobody should be using at all, let alone via import *), but it seems wrong that the warning was emitted at all given that contextlib2.ContextStack is listed in https://github.com/jazzband/contextlib2/blob/master/dev/mypy.allowlist (at least, it was when I was getting this error. I commented it out later after mypy complained that the entry wasn't needed anymore - it may be that mypy's type inferencing has improved to the point where it can now derive the ContextStack method signatures from the ExitStack ones)

Adding ContextStack just to __all__ in the stub without adding an actual spec for it didn't make the error go away.

Expected Behavior

Listing contextlib2.ContextStack in the mypy.allowlist file would make mypy never care about that name, including in the __all__ check.

Actual Behavior

__all__ error as shown above

Your Environment

  • Mypy version used: mypy 1.8.0 (compiled: no)
  • Mypy command-line: python -m mypy.stubtest --allowlist dev/mypy.allowlist contextlib2
  • Mypy configuration options from mypy.ini (and other config files): defaults (no config files)
  • Python version used: mostly Python 3.12, but reproduced on 3.8 -> 3.11 with tox
@ncoghlan ncoghlan added the bug mypy got something wrong label May 22, 2024
@hauntsaninja
Copy link
Collaborator

hauntsaninja commented May 22, 2024

Thanks for the issue. As of mypy 1.9, if there's an error with contextlib2.__all__, the entry in the allowlist should be contextlib2.__all__ (before that, it was just contextlib2, see #14217).

I'm inclined to keep it this way, since__all__ is the actual symbol that has the discrepancy. E.g. if there's something about ContextStack that stubtest can't understand, that feels different enough from whether or not __all__ is correct that you wouldn't want to silence everything

@ncoghlan
Copy link
Author

Given that the resolution in my case was "If it isn't important enough to include in the stub file, it doesn't make sense to leave it in __all__ either", I'm finding it hard to think of a case where it actually would be critical that a name that didn't typecheck absolutely had to be included in __all_.

On that basis, I agree the current behaviour is fine (either names that don't typecheck have to be left out of the runtime __all__ as well as the typechecked one, or else the __all__ check has to be turned off entirely for that module)

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

No branches or pull requests

2 participants