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

Assertion error, importing library with typehints #8481

Closed
Stonedestroyer opened this issue Mar 3, 2020 · 14 comments · Fixed by #11632
Closed

Assertion error, importing library with typehints #8481

Stonedestroyer opened this issue Mar 3, 2020 · 14 comments · Fixed by #11632

Comments

@Stonedestroyer
Copy link

Stonedestroyer commented Mar 3, 2020

Note: if you are reporting a wrong signature of a function or a class in
the standard library, then the typeshed tracker is better suited
for this report: https://github.com/python/typeshed/issues

Please provide more information to help us understand the issue:

  • Are you reporting a bug, or opening a feature request?
    Bug, assertionerror
  • Please insert below the code you are checking with mypy,
    or a mock-up repro if the source is private. We would appreciate
    if you try to simplify your case to a minimal repro.

Library used Rapptz/discord.py#1497.

Code

import discord

Running mypy --strict file.py crashes it but if you do normal run without strict and then strict it works until you do strict again. It seems doing strict off the bat without having ran without --strict previous crashes it.

Traceback

Traceback (most recent call last):
  File "C:\Python38\lib\runpy.py", line 193, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Python38\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\stonedestroyer\bot\.venv\Scripts\mypy.exe\__main__.py", line 7, in <module>
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\__main__.py", line 8, in console_entry
    main(None, sys.stdout, sys.stderr)
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\main.py", line 89, in main
    res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\build.py", line 180, in build
    result = _build(
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\build.py", line 249, in _build
    graph = dispatch(sources, manager, stdout)
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\build.py", line 2649, in dispatch
    process_graph(graph, manager)
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\build.py", line 2956, in process_graph
    process_stale_scc(graph, scc, manager)
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\build.py", line 3074, in process_stale_scc
    graph[id].write_cache()
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\build.py", line 2266, in write_cache
    new_interface_hash, self.meta = write_cache(
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\build.py", line 1441, in write_cache
    data = tree.serialize()
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\nodes.py", line 302, in serialize
    'names': self.names.serialize(self._fullname),
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\nodes.py", line 3097, in serialize
    data[key] = value.serialize(fullname, key)
  File "c:\stonedestroyer\bot\.venv\lib\site-packages\mypy\nodes.py", line 3033, in serialize
    assert not isinstance(self.node, PlaceholderNode)
AssertionError

Same issue with latest mypy from master.

@Stonedestroyer Stonedestroyer changed the title [Crash] [Crash] Assertion error, importing library with typehints Mar 3, 2020
@ilevkivskyi
Copy link
Member

Unfortunately I can't reproduce this, mypy just doesn't find the stubs (even though I installed from the right branch). It however looks like something bad (and it seems to me we had seen another similar report recently), so worth more investigation.

@ilevkivskyi ilevkivskyi changed the title [Crash] Assertion error, importing library with typehints Assertion error, importing library with typehints Mar 6, 2020
@bryanforbes
Copy link
Contributor

@ilevkivskyi let me know if you need help reproducing this. I'm the maintainer of the stubs and I'm experiencing the same issue on 0.770.

@ethanhs
Copy link
Collaborator

ethanhs commented Mar 13, 2020

I can repro this crash, I needed to install aiohttp and websockets from requirements.txt for it to happen. It seems that the assertion failure happens for discord.channel.BaseUser. My guess is there a large import cycle in the channel/user pairing that is causing the issue, but I am not sure (I'm afraid I don't have more time to investigate this further).

@bryanforbes
Copy link
Contributor

I was able to fix this crash. webhook.pyi was importing discord.state.ConnectionState and this was causing the import cycles. I hadn't stubbed ConnectionState because it's not part of the public API, so I stubbed it as an opaque structure so Webhook.from_state() can use it. @Stonedestroyer, update your checkout to bryanforbes/discord.py@ea11c83 and let me know if the problem persists.

@bryanforbes
Copy link
Contributor

I did find an issue with an import cycle using from .channel import * in discord/__init__.pyi. I had to fix it by specifying exactly what to import from discord.channel (@Stonedestroyer you'll want to update your checkout to bryanforbes/discord.py@30fb161 ). This seems to have been introduced in 0.770.

@ethanhs
Copy link
Collaborator

ethanhs commented Apr 5, 2020

@bryanforbes thank you for investigating this further! If you could find a more minimal reproduction that would be greatly appreciated.

@bryanforbes
Copy link
Contributor

@ethanhs I'll see what I can do

@bryanforbes
Copy link
Contributor

bryanforbes commented Apr 5, 2020

@ethanhs I was able to reproduce the import cycle in the following gist: https://gist.github.com/bryanforbes/024ad256677d07faff0ed4dc5c5dd21a. In my reproduction project, I have a repro directory and I put all of those files into it. I then run .venv/bin/mypy repro/user.pyi and get the crash. If I change __init__.pyi to import all of the classes in channel.pyi individually (from .channel import TextChannel as TextChannel, ...), the crash goes away.

@ethanhs
Copy link
Collaborator

ethanhs commented Apr 6, 2020

@bryanforbes, thank you so much, that will be helpful in finding the source of the issue.

@bluebird75
Copy link

I am running into the same issue. I also suspect it is a kind of import cycle crash.

I have tried to reduce the example but by reducing it, the problem goes away. I also suspect some kind of import cycle because removing some module imports removes the problem.

You can easily reproduce it on my open source project :

d:\work\winpdb\winpdb1>mypy --strict rpdb\rpc.py
Traceback (most recent call last):
  File "c:\program files (x86)\python37-32\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "c:\program files (x86)\python37-32\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "C:\Program Files (x86)\Python37-32\Scripts\mypy.exe\__main__.py", line 7, in <module>
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\__main__.py", line 8, in console_entry
    main(None, sys.stdout, sys.stderr)
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\main.py", line 90, in main
    res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\build.py", line 181, in build
    sources, options, alt_lib_path, flush_errors, fscache, stdout, stderr, extra_plugins
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\build.py", line 254, in _build
    graph = dispatch(sources, manager, stdout)
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\build.py", line 2630, in dispatch
    process_graph(graph, manager)
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\build.py", line 2953, in process_graph
    process_stale_scc(graph, scc, manager)
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\build.py", line 3070, in process_stale_scc
    graph[id].write_cache()
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\build.py", line 2236, in write_cache
    self.manager)
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\build.py", line 1445, in write_cache
    data = tree.serialize()
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\nodes.py", line 303, in serialize
    'names': self.names.serialize(self._fullname),
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\nodes.py", line 3098, in serialize
    data[key] = value.serialize(fullname, key)
  File "c:\program files (x86)\python37-32\lib\site-packages\mypy\nodes.py", line 3034, in serialize
    assert not isinstance(self.node, PlaceholderNode)
AssertionError

@rraval
Copy link

rraval commented Dec 6, 2020

I managed to minimize the reproduction from https://github.com/bluebird75/winpdb, please see https://github.com/rraval/mypy-8481

This fails:

$ mypy --no-implicit-reexport pkg/a.py
Traceback (most recent call last):
  File "/home/rraval/.local/share/virtualenvs/encircle/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
  File "/home/rraval/.local/share/virtualenvs/encircle/lib/python3.6/site-packages/mypy/__main__.py", line 8, in console_entry
    main(None, sys.stdout, sys.stderr)
  File "mypy/main.py", line 90, in main
  File "mypy/build.py", line 180, in build
  File "mypy/build.py", line 254, in _build
  File "mypy/build.py", line 2630, in dispatch
  File "mypy/build.py", line 2953, in process_graph
  File "mypy/build.py", line 3070, in process_stale_scc
  File "mypy/build.py", line 2232, in write_cache
  File "mypy/build.py", line 1445, in write_cache
  File "mypy/nodes.py", line 303, in serialize
  File "mypy/nodes.py", line 3098, in serialize
  File "mypy/nodes.py", line 3034, in serialize
AssertionError

This seems to work:

$ mypy pkg/a.py
Success: no issues found in 1 source file

Version info:

$ mypy --version
mypy 0.790

My naive guess is that this is some interaction between --no-implicit-reexport and from pkg.a import *. Both seem to be necessary to make the assertion trigger occur.

@ngie-eign
Copy link

ngie-eign commented Oct 10, 2021

I just ran into this issue with a large $work project as well, and we're sort of stuck with an older version of mypy that supports 2.7 and 3.8:

$ mypy --version
mypy 0.770
$

Do folks have any suggestions for how the import cycle can be tracked down other than maybe use --pdb (which wouldn't work with CI build processes)?

PS. I know this isn't the last version that supports both; this is just the version I standardized things on 1.5 years ago when I added it to our development environment.

@ethanhs
Copy link
Collaborator

ethanhs commented Oct 11, 2021

@rraval thank you for the minimal reproduction, that is very helpful, I believe that the from pkg.a import * does indeed seem to be related to the cause.

@ngie-eign I would look for a patter such as an import cycle, where you do from module import *, where module conditionally imports some other thing with if TYPE_CHECKING.

@ngie-eign
Copy link

ngie-eign commented Oct 11, 2021

@ethanhs : thanks! I think I figured out the issue — it was environmental (unclean build; old cached files).

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants