search

python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.39k stars 2.82k forks source link

assert file not in self.flushed_files (without MYPY_PATH) #7510

Closed chriselion closed 2 years ago

chriselion commented 5 years ago

Bug

mypy_repro.tar.gz This is taken from https://github.com/Unity-Technologies/ml-agents/tree/develop - there are a lot of other large files in there, so this is only the relevant python files.

No assert (I still expect errors from my code though).

--ignore-missing-imports --disallow-incomplete-defs --namespace-packages --show-traceback --cache-dir=/dev/null --cache-dir was just to simplify the repro

See above

This seems similar to some other issues where this assert was hit (e.g. https://github.com/python/mypy/issues/4881, https://github.com/python/mypy/issues/6717) but in those, MYPY_PATH was set and here it's not.

JukkaL commented 5 years ago

I was able to reproduce the crash. If I comment out the assert, everything seems to work okay.

ilevkivskyi commented 5 years ago

Isn't this a duplicate of https://github.com/python/mypy/issues/4881?

JukkaL commented 5 years ago

Isn't this a duplicate of #4881?

Based on a quick investigation it wasn't clear to me whether the same fix would work for both issues. They are closely related at least. In this issue the key seems to be --namespace-packages, whereas #4881 doesn't use it but uses mypy_path.

The root cause seems to be the same -- mypy assigns multiple module names to a file, based on different package root paths.

chriselion commented 5 years ago

Thanks, glad the repro worked.

Given that this assert is the cause of a few issues, is it worth removing it (or maybe converting to a warning instead)?

JukkaL commented 5 years ago

Given that this assert is the cause of a few issues, is it worth removing it (or maybe converting to a warning instead)?

Yeah, maybe it should be turned into an error message. When we hit it something has gone wrong, but the crash is really unhelpful.

chriselion commented 5 years ago

OK, are you going to do that, or should I try it out?

JukkaL commented 5 years ago

@chriselion I'm going to be busy with other things for the next week and a half, so if you'd like to try it out, please go ahead!

chriselion commented 5 years ago

Hi @JukkaL, Sorry, finally trying to get back to this. First attempt is in https://github.com/python/mypy/pull/7567, although I could use some guidance on where to add tests.

chriselion commented 4 years ago

@JukkaL Were you ever able to look into this further? My PR to disable the assert got some pushback, and I'm still not sure how to diagnose and resolve what's causing a file to get processed multiple times.

I'm currently running mypy without --namespace-packages but this means it's missing out on flagging some legitimate problems that need to be fixed.

hauntsaninja commented 2 years ago

We added a check for "Source file found twice under different module names" a long time back that means this no longer reproes