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

request-cache with --no-implicit-reexport cause an AssertionError #11025

Closed
p-gag opened this issue Aug 25, 2021 · 2 comments · Fixed by #11632
Closed

request-cache with --no-implicit-reexport cause an AssertionError #11025

p-gag opened this issue Aug 25, 2021 · 2 comments · Fixed by #11632
Labels

Comments

@p-gag
Copy link

p-gag commented Aug 25, 2021

Crash Report

Only with --no-implicit-reexport, adding from requests_cache import CachedSession into any file, including an empty file cause this error: AssertionError: Definition of requests_cache.patcher.BaseCache is unexpectedly incomplete

Traceback

$ mypy --no-implicit-reexport app/test.py
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.9/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/mypy/__main__.py", line 11, in console_entry
    main(None, sys.stdout, sys.stderr)
  File "mypy/main.py", line 87, in main
  File "mypy/main.py", line 165, in run_build
  File "mypy/build.py", line 179, in build
  File "mypy/build.py", line 254, in _build
  File "mypy/build.py", line 2697, in dispatch
  File "mypy/build.py", line 3021, in process_graph
  File "mypy/build.py", line 3138, in process_stale_scc
  File "mypy/build.py", line 2288, in write_cache
  File "mypy/build.py", line 1475, in write_cache
  File "mypy/nodes.py", line 313, in serialize
  File "mypy/nodes.py", line 3149, in serialize
  File "mypy/nodes.py", line 3083, in serialize
AssertionError: Definition of requests_cache.patcher.BaseCache is unexpectedly incomplete```

To Reproduce

# test.py
from requests_cache import CachedSession

On the command line:

$ mypy --no-implicit-reexport test.py

Note: removing the .mypy_chache did not help.

Your Environment

  • request-cache used: 0.7.3
  • Mypy version used: 0.910
  • Mypy command-line flags: --no-implicit-reexport
  • Mypy configuration options from mypy.ini (and other config files): not used
  • Python version used: 3.9.1
  • Operating system and version: macOS Version 11.3.1
@p-gag p-gag added the crash label Aug 25, 2021
@JWCook
Copy link

JWCook commented Aug 28, 2021

Well, that's curious. I also reproduced this with the latest changes in the requests-cache master branch.

If I modify the import in requests_cache/patcher.py to

from .backends import BaseCache as BaseCache

The error goes away, but if I alias it to something else:

from .backends import BaseCache as CacheType

I get the same AssertionError.

I don't know what's going on there, but if you have suggestions for improving the type hints or exports in requests-cache, you're welcome to create an issue there: https://github.com/reclosedev/requests-cache/issues

@boehmseb
Copy link

Hi,

I just stumbled across the same problem. While I also have no idea what's going on, I can say that mypy only crashes from version 0.800 onwards. No crash with earlier versions. Maybe this helps identifying the problem.

boehmseb added a commit to se-sic/VaRA-Tool-Suite that referenced this issue Sep 20, 2021
requests_cache currently crashes mypy (python/mypy#11025) and the CVE provider doesn't work reliably anyway.
severo added a commit to huggingface/dataset-viewer that referenced this issue Oct 18, 2021
JukkaL pushed a commit that referenced this issue Dec 3, 2021
Fixes #8481
Fixes #9941
Fixes #11025
Fixes #11038

This is the unresolved crash that's been reported the most times, e.g., it's as easy 
to repro as `mypy --no-implicit-reexport -c 'import pytorch_lightning'` (or even 
`import torch`, with the right PyTorch version). I know of multiple popular Python 
packages that have made changes just to work around this crash. I would love to 
see this released, potentially along with #11630.

Thanks to @rraval for making a minimal repro!

The fix ended up being a one-liner, but it took me a bit to track down :-)

Since things worked with implicit reexport, I differentially patched in explicit reexport 
logic to narrow things down. This let me observe that if I hardcoded pkg.a.B to get 
reexported, we hit this branch, which clobbers the PlaceholderNode for pkg.c.B, 
which fixes things: 

https://github.com/python/mypy/blob/f79e7afec2c863c34d7a9b41ebb732dc26128fff/mypy/semanal.py#L2028

Which is a little weird — we shouldn't have a PlaceholderNode for pkg.c.B at all. But 
with a breakpoint in that branch, it was easy to notice that with `--no-implicit-reexport` 
pkg.a.B was first created with `module_public=True` (resulting in creation of a 
PlaceholderNode in pkg.c.B) and only on a later pass acquired `module_public=False`. 
So tracking down where pkg.a.B symbol was first created with `module_public=True` 
led me to this "obvious" bug.
ilevkivskyi pushed a commit that referenced this issue Dec 3, 2021
Fixes #8481
Fixes #9941
Fixes #11025
Fixes #11038

This is the unresolved crash that's been reported the most times, e.g., it's as easy 
to repro as `mypy --no-implicit-reexport -c 'import pytorch_lightning'` (or even 
`import torch`, with the right PyTorch version). I know of multiple popular Python 
packages that have made changes just to work around this crash. I would love to 
see this released, potentially along with #11630.

Thanks to @rraval for making a minimal repro!

The fix ended up being a one-liner, but it took me a bit to track down :-)

Since things worked with implicit reexport, I differentially patched in explicit reexport 
logic to narrow things down. This let me observe that if I hardcoded pkg.a.B to get 
reexported, we hit this branch, which clobbers the PlaceholderNode for pkg.c.B, 
which fixes things: 

https://github.com/python/mypy/blob/f79e7afec2c863c34d7a9b41ebb732dc26128fff/mypy/semanal.py#L2028

Which is a little weird — we shouldn't have a PlaceholderNode for pkg.c.B at all. But 
with a breakpoint in that branch, it was easy to notice that with `--no-implicit-reexport` 
pkg.a.B was first created with `module_public=True` (resulting in creation of a 
PlaceholderNode in pkg.c.B) and only on a later pass acquired `module_public=False`. 
So tracking down where pkg.a.B symbol was first created with `module_public=True` 
led me to this "obvious" bug.
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this issue Jan 20, 2022
…#11632)

Fixes python#8481
Fixes python#9941
Fixes python#11025
Fixes python#11038

This is the unresolved crash that's been reported the most times, e.g., it's as easy 
to repro as `mypy --no-implicit-reexport -c 'import pytorch_lightning'` (or even 
`import torch`, with the right PyTorch version). I know of multiple popular Python 
packages that have made changes just to work around this crash. I would love to 
see this released, potentially along with python#11630.

Thanks to @rraval for making a minimal repro!

The fix ended up being a one-liner, but it took me a bit to track down :-)

Since things worked with implicit reexport, I differentially patched in explicit reexport 
logic to narrow things down. This let me observe that if I hardcoded pkg.a.B to get 
reexported, we hit this branch, which clobbers the PlaceholderNode for pkg.c.B, 
which fixes things: 

https://github.com/python/mypy/blob/f79e7afec2c863c34d7a9b41ebb732dc26128fff/mypy/semanal.py#L2028

Which is a little weird — we shouldn't have a PlaceholderNode for pkg.c.B at all. But 
with a breakpoint in that branch, it was easy to notice that with `--no-implicit-reexport` 
pkg.a.B was first created with `module_public=True` (resulting in creation of a 
PlaceholderNode in pkg.c.B) and only on a later pass acquired `module_public=False`. 
So tracking down where pkg.a.B symbol was first created with `module_public=True` 
led me to this "obvious" bug.
mattstern31 added a commit to mattstern31/datasets-server-storage-admin that referenced this issue Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants