mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Restrictions on MYPYPATH

Open orenbenkiki opened this issue 6 years ago • 26 comments

Given that PYTHONPATH contains .../pythonX.Y/site-packages And a package that contains some private 3rd party stubs under .../pythonX.Y/site-packages/foo/some_stubs

When setting MYPYPATH to .../pythonX.Y/site-packages/foo/some_stubs

Then mypy gives the error .../pythonX.Y/site-packages is in the MYPYPATH. Please remove it..

This is wrong because:

  1. MYPYPATH does not include ../pythonX.Y/site-packages. It does include .../pythonX.Y/site-packages/foo/some_stubs but that is a very different directory.

  2. Due to this error mypy refuses to use the stubs listed in MYPYPATH in this case.

This probably isn't intentional? The relevant PEP explicitly allows for MYPYPATH and does not seem to mention any restriction on the location of stubs added via MYPYPATH.

It is possible to work around the problem by copying (or linking) the stubs to a directory which is not under ../pythonX.Y/site-packages, say into ~/.foo-stubs and adding this path to MYPYPATH.

orenbenkiki avatar Oct 02 '19 15:10 orenbenkiki

I think this is intentional.

Does it work to add a py.typed to the stubs package?

msullivan avatar Oct 02 '19 17:10 msullivan

Hmmm, I'll try tomorrow.

What would be the motivation of forbidding this?

orenbenkiki avatar Oct 02 '19 17:10 orenbenkiki

The semantics of MYPYPATH are entirely up to the type checker using it. In addition, we do not want to add anything in site packages to the MYPYPATH, as that can be problematic. In general it is better to add a py.typed file and not use MYPYPATH for installed packages.

emmatyping avatar Oct 02 '19 19:10 emmatyping

The reason for this check is that almost any time someone tries to include site-packages in $MYPYPATH it causes problems -- it just is almost never what you want. Hence the explicit check (easy to find in the mypy source by grepping for "Please remove it"). I don't think the use case of stubs distributed in a subdirectory of an installed package is important enough to make an exception, and your workaround sounds better -- though using a separate stubs package as described in PEP 561 would probably be better.

gvanrossum avatar Oct 02 '19 22:10 gvanrossum

The semantics of MYPYPATH are entirely up to the type checker using it.

Makes sense.

In addition, we do not want to add anything in site packages to the MYPYPATH, as that can be problematic.

And:

The reason for this check is that almost any time someone tries to include site-packages in $MYPYPATH it causes problems -- it just is almost never what you want.

Fair enough. I would then suggest the error message be improved to "MYPYPATH contains the path: ... which is under the site packages path: ..." - this would make it clear what the intention is. I found the current error message to be confusing as it claimed MYPYPATH contained the site packages path itself, which it does not.

In general it is better to add a py.typed file and not use MYPYPATH for installed packages.

I don't see how adding py.typed helps here.

My scenario is that I have one package providing common utilities and a few other packages that use this utilities package. One of the services the utilities package offers is some very partial numpy & pandas stubs which are tailored to a specific problem domain. This is "good enough" until proper stubs are published (holding fingers).

your workaround sounds better -- though using a separate stubs package as described in PEP 561 would probably be better.

My workaround is to create a symbolic link from the sub-directory in the site packages to a local .my-stubs directory and adding that into MYPYPATH. This is automated by the utilities package. This works but I was uncomfortable about going against the "spirit of the law" here.

I do not want to take my very partial, very domain-specific stubs and claim the numpy-stubs and pandas-stubs package names. That would signal an intent to evolving the packages towards becoming the end-all-be-all stub packages for numpy and pandas and that isn't something I can commit to.

orenbenkiki avatar Oct 03 '19 07:10 orenbenkiki

I agree with @orenbenkiki .

The error message is not helpful, I had to Google for 15 mins to find this rationale.

ghost avatar Mar 03 '20 13:03 ghost

@woolinsilver-kainos agreed the error message could be improved. Would you like to submit a PR to improve it?

emmatyping avatar Mar 03 '20 21:03 emmatyping

This is occurring to me and it quite bad as this was my generic workaround for #5701 when using nix/nixos (see other issue for details). Now, I'm stuck adding each problematic package to MYPYPATH to avoid the "error: Cannot find module named 'blah'".

jraygauthier avatar May 11 '20 20:05 jraygauthier

Would it be possible to expose a flag / configuration to allow this, at least until a proper solution is found for #5701 ?

jraygauthier avatar May 11 '20 20:05 jraygauthier

Does the symlink workaround work for you?

I'm not wholly opposed to adding a flag I guess.

msullivan avatar May 19 '20 00:05 msullivan

The symbolic link works for me in my specific setup, but it is a pretty fragile solution and a bit of a hack.

Are you suggesting a command line flag that would be allowed to add an entry to the search path, even if this entry is under the site packages directory? That would solve the problem, but it seems if you are going that way, you might as well lift the restriction for normal entries in MYPYPATH in the 1st place...

All that aside, the error message is still too cryptic IMVO.

orenbenkiki avatar May 20 '20 15:05 orenbenkiki

@msullivan :

Does the symlink workaround work for you?

It might do. Will have to try. However, I do not like much the stateful approach as I will be left with some dangling / stable / potentially harmful symlink in my working directory.

Seems to me like the original approach (@orenbenkiki took) of adding individual packages from site-package (and not the whole thing) should be allowed by default without requiring hacks of the sort.

@orenbenkiki :

Are you suggesting a command line flag that would be allowed to add an entry to the search path, even if this entry is under the site packages directory?

This was my suggestion indeed.

but it seems if you are going that way, you might as well lift the restriction for normal entries in MYPYPATH in the 1st place...

I do not agree with this point as the point of the restriction is to warn the user about a potential error on his part. Now, the flag tells: I understand this, however, I know what I'm doing. You explicitly opt-in for the free pass.

jraygauthier avatar May 20 '20 18:05 jraygauthier

@orenbenkiki : By the way, not sure why you closed the ticket as it seems there was no proper resolution?

jraygauthier avatar May 20 '20 18:05 jraygauthier

@jraygauthier : You are right. Re-opening.

There are actually two sub-issues:

  • Just fixing the error message to not be confusing.

  • Actually providing the functionality.

I'm not sure how explicitly adding to MYPYPATH a directory that exists inside a package in site-packages is a potential error. What is the scenario where this is an actual error?

Note in my case, it isn't the root directory of a package, it is a sub-directory within some package.

That said, if a command line flag is deemed as more explicit (this being Python and explicitness being a value), sure, that would solve the second sub-issue.

orenbenkiki avatar May 21 '20 06:05 orenbenkiki

I am a little lost on whats the issue at hand, but I do not have a MYPYPATH defined anywhere and I get the same error.

sjosegarcia avatar May 26 '20 22:05 sjosegarcia

@sjosegarcia where are you running mypy from? if it is in site packages then you will get this error

emmatyping avatar May 26 '20 23:05 emmatyping

I am running it within my virtualenv @ethanhs .

sjosegarcia avatar May 26 '20 23:05 sjosegarcia

same here, running it from virtualenvwrapper

shackra avatar Jun 09 '20 21:06 shackra

I think the exclusion of .../pythonX.Y/site-packages from MYPYPATH is unreasonable. It excludes all python apps which are installed in site-packages i.e. all python apps installed system-wide. The setup.py file of MyPy itself installs into site-packages, yet other apps installed the same way (in my case thonny) raise this error.

There is a comment above that "it just is almost never what you want" but I've not seen any explanation why it is not what I want. I want the warning about it even less.

BTW the "site-packages is in the MYPYPATH" message is passed up via thonny as a WARNING, yet the MyPy code immediately does a sys.exit(1) - which doesn't seen right (exiting with an error).

This issue has been around unresolved for a long time. Until it is resolved one way or another could the site-packages restriction be suspended?

cwilling avatar Feb 09 '21 13:02 cwilling

FWIW, I am experiencing this just from calling flake8-mypy via flake8 via pre-commit via tox OR when running flake8 from the command line. I separately tried using a mypy pre-commit hook directly and it does NOT exhibit the issue. Running mypy . also does NOT exhibit the issue. (I can just use the mypy pre-commit-hook but I was looking into how the flake8 version outputted...I do like that it tracks statistics regarding each type of issue.)

In case relevant, excerpts from my configs are below. (Also if this is a PEBKAC, pleaes let me know, thank you. I hope it isn't...)

Flake8 config:

Click to expand!
 [flake8]
    indent-size = 4
    max-line-length = 89
    max-complexity = 18
    show-source = true
    statistics = true
    doctests = true
    ignore =
        # continuation line over-indented for hanging indent
        E126,
        # closing bracket is missing indentation
        E133,
        # multiple spaces after ','
        E241,
        # whitespace before ',', ';', or ':'
        E203,
        # too many leading '#' for block comment
        E266,
        # 'from module import *' used; unable to detect undefined names
        F403,
        # line break before binary operator
        W503,
        # line break after binary operator
        W504,
    select =
        # provided by flake8-bugbear
        # https://github.com/PyCQA/flake8-bugbear
        #   Various agreed upon warnings (B0)
        #   Various controversial warnings (B9)
        B,
        # explicitly enable B9
        B9,
        # provided by flake8-commas
        # https://github.com/PyCQA/flake8-commas
        #   Missing Commas (C81)
        # provided by mccabe
        # https://github.com/PyCQA/mccabe
        #   Max complexity exceded (C901)
        C,
        # provided by flake8-docstrings (pydocstyle)
        # https://www.pydocstyle.org/en/stable/error_codes.html
        #   Missing Docstrings (D1)
        #   Whitespace Issues (D2)
        #   Quotes Issues (D3)
        #   Docstring Content Issues (D4)
        D,
        # provided by pycodestyle
        # https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes
        #   Indentation (E1)
        #   Whitespace (E2)
        #   Blank line (E3)
        #   Import (E4)
        #   Line length (E5)
        #   Statement (E7)
        #   Runtime (E9)
        E,
        # provided flake8
        # https://flake8.pycqa.org/en/latest/user/error-codes.html
        F,
        # provided by flake8-mypy
        # https://github.com/ambv/flake8-mypy
        #   all type related errors
        T4,
        # provided by pycodestyle
        # https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes
        #   Indentation warning (W1)
        #   Whitespace warning (W2)
        #   Blank line warning (W3)
        #   Line break warning (W5)
        #   Deprecation warning (W6)
        W,
        # provided by flake8-pyi
        # https://github.com/PyCQA/flake8-pyi
        #   Type Hinting Errors in .pyi files (Y0)
        Y,

Pre-commit config:

Click to expand!
  - repo: https://github.com/PyCQA/flake8
    rev: 4.0.1
    hooks:
      - id: flake8
        additional_dependencies: [
          flake8-bugbear,
          flake8_commas,
          flake8-docstrings,
          flake8-mypy,
          flake8-pyi,
          pytest,
        ]

  - repo: https://github.com/pre-commit/mirrors-mypy
    rev: v0.902
    hooks:
      - id: mypy
        args: [--strict]
        additional_dependencies: [pytest]

Tox config:

Click to expand!
[testenv:lint]
skip_install = True
deps =
    pre-commit
commands =
    pre-commit run --all-files

MarximusMaximus avatar Apr 22 '22 00:04 MarximusMaximus

Coming back to this:

a) this issue is no longer nearly as important now that #5701 is resolved

b) maybe we can lift the restriction on MYPYPATH entirely, since the main reason it was put in place (IIRC) was to avoid accidentally picking up typing.py installed through pip and causing mypy to crash

emmatyping avatar Jun 05 '22 11:06 emmatyping

Ok I think with https://github.com/python/mypy/pull/13155 and https://github.com/python/mypy/pull/11143 we can probably lift these restrictions.

emmatyping avatar Aug 13 '22 09:08 emmatyping

Please do. I have a case where I'm using pylsp LSP server and need to specify same directory for both PYTHONPATH and MYPYPATH to make both Jedi engine and Mypy happy.

If I only add the directory to PYTHONPATH then Mypy will complain with module is installed, but missing library stubs or py.typed marker.

If I add to both then mypy will error out with x is in the MYPYPATH. Please remove it.

I guess the expectation is that I only add that code to PYTHONPATH and add stubs or py.typed but I can't do that for all the code that is not controlled by me that I want to import.

rchl avatar Sep 16 '22 20:09 rchl

@rchl I think you may be running into a regression in 0.971 that causes mypy to spuriously complain about MYPYPATH stuff. This regression was caused by #11143. See https://github.com/python/mypy/issues/13214 for more details. This has been fixed on master for a long time, unfortunately a bug release wasn't made for it.

hauntsaninja avatar Sep 16 '22 21:09 hauntsaninja

Oh. I am indeed using 0.971 and switching to 0.961 seems to be fixing my issue. Thanks!

rchl avatar Sep 16 '22 21:09 rchl

Apologies that your time got wasted on this!

hauntsaninja avatar Sep 16 '22 21:09 hauntsaninja

I'm closing this, since the history is sort of long and complicated. If we want to support this, I'd rather see a set of use cases in the last two years. Please open a new issue :-)

hauntsaninja avatar Jul 03 '24 08:07 hauntsaninja