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

Strange name substitution on upgrading to >= 0.900 from 0.812 #10661

Closed
choogeboom opened this issue Jun 17, 2021 · 18 comments · Fixed by #11708
Closed

Strange name substitution on upgrading to >= 0.900 from 0.812 #10661

choogeboom opened this issue Jun 17, 2021 · 18 comments · Fixed by #11708
Labels
bug mypy got something wrong

Comments

@choogeboom
Copy link

Bug Report

I want to apologize in advance for the somewhat vague nature of this bug report. I've tried for hours in order to create a reproducible example, and have failed. Unfortunately, the bug occurs in some proprietary code, so I can't release it to you all to look at.

I went to upgrade my project to use mypy 0.902 and after fixing all the expected errors related to third party stub libraries, I am left with the following two errors:

buyfair/model/Warehouse.py:312: error: Invalid index type "buyfair.model.Warehouse.Warehouse" for "Dict[Warehouse, WarehouseProduct]"; expected type "buyfair.model.warehouse.Warehouse"
buyfair/model/Product.py:222: error: Argument 1 to "obeys_maximums" of "WarehouseSource" has incompatible type "Dict[buyfair.model.Product.Product, buyfair.model.Product.Unit]"; expected "Dict[buyfair.model.product.Product, buyfair.model.product.Unit]"

These errors are really strange, because there isn't any class called buyfair.model.Warehouse.Warehouse or buyfair.model.Product.Product. I shrugged my shoulders and added # type: ignore to each of these, and tried to run mypy again only to receive the following errors:

buyfair/model/warehouse.py:312: error: unused "type: ignore" comment
buyfair/model/product.py:222: error: unused "type: ignore" comment

I've set warn_unused_ignores=False and will move on, but figured this is at least worth reporting.

To Reproduce

Like I said above, I tried to reproduce this error in a smaller more self-contained example for a few hours, but wasn't successful. I'll try to give more details around the context in the error messages above:

buyfair/model/warehouse.py

from typing import Dict, AbstractSet, TYPE_CHECKING

if TYPE_CHECKING:
    from buyfair.model.product import Product, Unit

class Warehouse:
    warehouse_sources: Dict["Source", "WarehouseSource"]

class WarehouseProduct:
    some_bool_value: bool

class WarehouseGroup(AbstractSet[Warehouse]):

    def some_bool_value(self, product: "Product") -> bool:
        return any(product.warehouse_products[warehouse].some_bool_value for warehouse in self)  # <-- problematic line


class WarehouseSource:
    def obeys_maximums(self, products: Dict["Product", "Unit"]) -> bool:
        ...

buyfair/model/product.py

from typing import Dict, NewType, TYPE_CHECKING

if TYPE_CHECKING:
    from buyfair.model.warehouse import Warehouse, WarehouseProduct, WarehouseSource

Unit = NewType("Unit", int)

class Product:
    warehouse_products: "Dict[Warehouse, WarehouseProduct]"
    
    def some_other_bool_value(warehouse: "Warehouse") -> bool:
       warehouse_source:  "WarehouseSource" = ...
        products = {self: Unit(0)}
        if warehouse_source.obeys_maximums(products):  # <-- Other problematic line
            ...

In both these cases, mypy is replacing the module name with the class name.

buyfair.model.product.Product becomes buyfair.model.Product.Product and buyfair.model.warehouse.Warehouse becomes buyfair.model.Warehouse.Warehouse

Expected Behavior

In both these cases, the types work out just fine, and mypy should recognize the types just fine.

Actual Behavior

mypy fails to recognize the types correctly and creates a new made up type.

(Write what happened.)

Your Environment

  • Mypy version used: 0.900, 0.901, 0.902
  • Mypy command-line flags:
  • Mypy configuration options from mypy.ini (and other config files):
    python_version=3.6
    warn_unused_ignores=False
    check_untyped_defs=True
    strict_optional=False
    
  • Python version used: 3.6
  • Operating system and version: Centos 7.5 (running inside docker)
@choogeboom choogeboom added the bug mypy got something wrong label Jun 17, 2021
@ethanhs
Copy link
Collaborator

ethanhs commented Jun 17, 2021

Thank you for giving a detailed bug report. My guess is there is an issue with some kind of import cycle confusing the analyzer, but that wouldn't make sense why it would think the classes contain themselves...

Could you check if you can get attributes off of the replaced types? Also perhaps try using reveal_type() before?

@choogeboom
Copy link
Author

I added reveal_type as follows:

in buyfair/model/warehouse.py

    def some_bool_value(self, product: "Product") -> bool:
        for warehouse in self:
            reveal_type(warehouse)
        return any(product.warehouse_products[warehouse].some_bool_value for warehouse in self)  # <-- problematic line

and in buyfair/model/product.py

        reveal_type(self)
        products = {self: Unit(0)}
        if warehouse_source.obeys_maximums(products):  # <-- Other problematic line

Strangely, mypy reveals each line twice: once correctly, and once incorrectly:

buyfair/model/warehouse.py:313: note: Revealed type is "buyfair.model.warehouse.Warehouse*"
buyfair/model/product.py:221: note: Revealed type is "buyfair.model.product.Product"
buyfair/model/Warehouse.py:313: note: Revealed type is "buyfair.model.Warehouse.Warehouse*"
buyfair/model/Product.py:221: note: Revealed type is "buyfair.model.Product.Product"

Could you check if you can get attributes off of the replaced types?

I'm not sure what you mean by that

@JelleZijlstra
Copy link
Member

Interesting that one case has lowercase warehouse.py and one has uppercase Warehouse.py. That's probably a clue to where the problem is.

@NeilGirdhar
Copy link
Contributor

Maybe his file system is case insensitive, but some functions are normalizing filenames?

@choogeboom
Copy link
Author

This is running in CentOS, so the file system is definitely case sensitive

@JelleZijlstra
Copy link
Member

And just to make sure, you don't have both a warehouse.py and a Warehouse.py?

@choogeboom
Copy link
Author

that is correct!

@JelleZijlstra
Copy link
Member

Strange, then I wonder where mypy gets the uppercase filenames from. If your filesystem isn't case-insensitive, maybe something in the mypy cache is?

@paulcwatts
Copy link

I'm experiencing this issue as well in my Django projects.

  • Python version: 3.7
  • mypy version: 0.910
  • Operating system: Debian, Python:3.7-slim base image, macOS 11.4 host. See notes below.

Configuration:

[mypy]
python_version = 3.7
disallow_untyped_defs = True
ignore_missing_imports = True
warn_redundant_casts = True
warn_unused_ignores = True

I'm getting pretty much what @choogeboom is getting:

users/models/User.py:170: error: "User" has no attribute "external"

Even with the strange duplicate reveal_type issue.

users/models/user.py:170: note: Revealed type is "users.models.user.User"
users/models/User.py:170: note: Revealed type is "users.models.User.User"

Funny thing is, my coworkers that are using Windows as host do not have the same error. It also doesn't occur on our CI machines.

I also have an inkling that it has something to do with importing these classes through __init__.py, though I can't be certain yet.

@paulcwatts
Copy link

paulcwatts commented Jun 24, 2021

What I have is three files:

  • users/models/a.py
  • users/models/b.py
  • users/models/c.py

And a file users/models/__init__.py:

from .a import ModelA
from .b import ModelB
from .c import ModelC

__all__ = ["ModelA", "ModelB", "ModelC"]

All of my code imports from users.models, not any of the submodules. I can confirm that if I combine all of these files into one users/models.py, the error goes away.

@choogeboom
Copy link
Author

We have a similar structure in our codebase.

Everything outside models only imports models, but internally, the subpackages/submodules do import each other to prevent cyclical dependencies.

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 24, 2021

Can you give the full module hierarchy + all the names defined in the relevant modules? Also I'd like to see the relevant from .x import statements.

Does the removal of __all__ have any affect?

Is there both a class mod.Thing and a module mod.thing (lower case version of the first one)?

Can you double check that your file system is not case insensitive? Even on Linux it's possible to have case insensitive file systems, I think.

@paulcwatts
Copy link

This is about as simple as a test as I can come up with: https://github.com/paulcwatts/mypytest

It definitely is strange. I can't repro the issue with just the code -- it has to be within the Docker container. What's more, the code itself has to be mounted as a volume.

First build the image:

docker build -t mypytest .

Then, if you run mypy with code in the image:

% rm -rf .mypy_cache/; docker run --rm mypytest mypy .                                 
Success: no issues found in 7 source files

If you run mypy with the code mounted on a volume:

% rm -rf .mypy_cache/; docker run -v $(pwd):/app --rm mypytest mypy .
testapp/models/A.py:6: error: "A" has no attribute "b_set"
Found 1 error in 1 file (checked 7 source files)

It seems like it might have something to do with what gets stored in the mypy cache:

% rm -rf .mypy_cache/; docker run -v $(pwd):/app --rm mypytest mypy .
testapp/models/A.py:6: error: "A" has no attribute "b_set"
Found 1 error in 1 file (checked 7 source files)
 % ls -l .mypy_cache/3.7/testapp/models 
total 56
-rw-r--r--  1 paul  staff  4163 Jun 24 13:26 B.data.json
-rw-r--r--  1 paul  staff  1822 Jun 24 13:26 B.meta.json
-rw-r--r--  1 paul  staff  1491 Jun 24 13:26 __init__.data.json
-rw-r--r--  1 paul  staff  1508 Jun 24 13:26 __init__.meta.json
-rw-r--r--  1 paul  staff  4000 Jun 24 13:26 a.data.json
-rw-r--r--  1 paul  staff  1644 Jun 24 13:26 a.meta.json

As opposed to:

% docker run  --rm -it mypytest sh         
# cd /app 
# mypy .
Success: no issues found in 7 source files
# ls -l .mypy_cache/3.7/testapp/models/
total 24
-rw-r--r-- 1 root root 1491 Jun 24 19:22 __init__.data.json
-rw-r--r-- 1 root root 1508 Jun 24 19:22 __init__.meta.json
-rw-r--r-- 1 root root 4000 Jun 24 19:22 a.data.json
-rw-r--r-- 1 root root 1644 Jun 24 19:22 a.meta.json
-rw-r--r-- 1 root root 3940 Jun 24 19:22 b.data.json
-rw-r--r-- 1 root root 1822 Jun 24 19:22 b.meta.json
#     

@paulcwatts
Copy link

And to answer your question, @JukkaL , the host file system is case insensitive (APFS), but the container file system (ext4) is not. That might have something to do with it?

@paulcwatts
Copy link

It looks like this might have something to do with it? #10093 .

"On Linux we skip all logic, since the file system is almost always case sensitive. "

This is one case where that assumption doesn't apply.

@SteVwonder
Copy link

Just wanted to chime in that I ran into this exact issue, and I believe I have independently confirmed both of @paulcwatts 's conclusions.

I was working on some python code on a mac with a case-insensitive filesystem using VS Code's container remote extension. The VS Code remote extension automatically bind-mounts the source code on the case-insensitive mac into the linux container. Issue on the related repo with the errors: ExaWorks/psij-python#47

I tested the same codebase with mypy v0.800 (before the linked PR #10093 was merge) and the issue disappeared.

It seems that in order to handle these edge cases, there needs to be someway to turn off the "platform == Linux" short-circuit for the case-sensitivity checking, since a case-insensitive filesystem could still be mounted into a linux VM/container. If anyone has any ideas/pointers for how to accomplish that (I'm unfamiliar with mypy's codebase/configuration/practices), I'd be willing to take a stab at implementing it and submitting a PR. Would a command-line flag be acceptable? Any other ideas?

@SteVwonder
Copy link

I tested the same codebase with mypy v0.800 (before the linked PR #10093 was merge) and the issue disappeared.

Realizing that this test wasn't all that rigorous, I went ahead and grabbed the latest mypy from git and the problem is still around:

(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python$ git clone https://github.com/python/mypy.git
Cloning into 'mypy'...
<snip>
(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python$ cd mypy/
(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python/mypy$ python3 -m pip install -U .
Processing /workspaces/psi-j-python/mypy
<snip>
Successfully installed mypy-0.920+dev.5adb0a05c157d9f5afbdb048a6d06a3234586ab6 tomli-1.1.0
(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python/mypy$ which mypy
/workspaces/psi-j-python/venv-vscode-focal/bin/mypy
(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python/mypy$ mypy --version
mypy 0.920+dev.5adb0a05c157d9f5afbdb048a6d06a3234586ab6
(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python/mypy$ cd ..
(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python$ make typecheck
mypy --config-file=.mypy --strict src tests
src/psi/j/Job.py:117: error: Argument 1 to "cancel" of "JobExecutor" has incompatible type "psi.j.Job.Job"; expected "psi.j.job.Job"
Found 1 error in 1 file (checked 29 source files)
make: *** [Makefile:12: typecheck] Error 1

Removed the linux shortcircuit (see git diff below) and the problem went away:

(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python/mypy$ git --no-pager diff
diff --git a/mypy/fscache.py b/mypy/fscache.py
index 37f50f622..c0598fd6c 100644
--- a/mypy/fscache.py
+++ b/mypy/fscache.py
@@ -199,9 +199,6 @@ class FileSystemCache:
 
         The caller must ensure that prefix is a valid file system prefix of path.
         """
-        if sys.platform == "linux":
-            # Assume that the file system on Linux is case sensitive
-            return self.isfile(path)
         if not self.isfile(path):
             # Fast path
             return False
(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python$ cd mypy/
(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python/mypy$ python3 -m pip install -U .
Processing /workspaces/psi-j-python/mypy
<snip>
Successfully installed mypy-0.920+dev.5adb0a05c157d9f5afbdb048a6d06a3234586ab6.dirty
(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python/mypy$ cd ..
(venv-vscode-focal)  fluxuser@1f7fe27de5f4:/workspaces/psi-j-python$ make typecheck
mypy --config-file=.mypy --strict src tests
Success: no issues found in 29 source files

So I think that is pretty conclusive.

If anyone has any ideas/pointers for how to accomplish that (I'm unfamiliar with mypy's codebase/configuration/practices), I'd be willing to take a stab at implementing it and submitting a PR. Would a command-line flag be acceptable? Any other ideas?

To be more specific, would a environment-specific configuration option like assume_case_insensitive_fs be appropriate to add to a mypy configuration file? My (very ignorant) impression is that mypy configuration options are meant to be checked into version control and applied to an entire codebase as opposed to a particular dev environment. Would a command line flag be more appropriate? Do all command line flags have a corresponding config option in mypy?

Maybe the best option is to just remove the short-circuit all together so that this issue is guaranteed to not occur, regardless of system configuration?

@zidarsk8
Copy link

And we have the same issue in docker on a mac, and reproducible with https://github.com/zidarsk8/mypy-mac-docker

hauntsaninja pushed a commit to hauntsaninja/mypy that referenced this issue Dec 10, 2021
JukkaL pushed a commit that referenced this issue Dec 13, 2021
Originally added in #10093

Fixes #11690, fixes #10661, fixes #10822

Co-authored-by: hauntsaninja <>
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this issue Jan 20, 2022
Originally added in python#10093

Fixes python#11690, fixes python#10661, fixes python#10822

Co-authored-by: hauntsaninja <>
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

Successfully merging a pull request may close this issue.

8 participants