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

Restrictions on MYPYPATH #7604

Open
orenbenkiki opened this issue Oct 2, 2019 · 26 comments
Open

Restrictions on MYPYPATH #7604

orenbenkiki opened this issue Oct 2, 2019 · 26 comments

Comments

@orenbenkiki
Copy link

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.

@msullivan
Copy link
Collaborator

I think this is intentional.

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

@orenbenkiki
Copy link
Author

Hmmm, I'll try tomorrow.

What would be the motivation of forbidding this?

@ethanhs
Copy link
Collaborator

ethanhs commented Oct 2, 2019

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.

@gvanrossum
Copy link
Member

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.

@orenbenkiki
Copy link
Author

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.

@ghost
Copy link

ghost commented Mar 3, 2020

I agree with @orenbenkiki .

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

@ethanhs
Copy link
Collaborator

ethanhs commented Mar 3, 2020

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

@jraygauthier
Copy link

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
Copy link

jraygauthier commented May 11, 2020

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

@msullivan
Copy link
Collaborator

Does the symlink workaround work for you?

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

@orenbenkiki
Copy link
Author

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.

@jraygauthier
Copy link

@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
Copy link

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

@orenbenkiki
Copy link
Author

orenbenkiki commented May 21, 2020

@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 orenbenkiki reopened this May 21, 2020
@sjosegarcia
Copy link

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.

@ethanhs
Copy link
Collaborator

ethanhs commented May 26, 2020

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

@sjosegarcia
Copy link

I am running it within my virtualenv @ethanhs .

@shackra
Copy link

shackra commented Jun 9, 2020

same here, running it from virtualenvwrapper

@cwilling
Copy link

cwilling commented Feb 9, 2021

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?

@MarximusMaximus
Copy link

MarximusMaximus commented Apr 22, 2022

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

@ethanhs
Copy link
Collaborator

ethanhs commented Jun 5, 2022

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

@ethanhs
Copy link
Collaborator

ethanhs commented Aug 13, 2022

Ok I think with #13155 and #11143 we can probably lift these restrictions.

@rchl
Copy link

rchl commented Sep 16, 2022

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.

@hauntsaninja
Copy link
Collaborator

@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 #13214 for more details. This has been fixed on master for a long time, unfortunately a bug release wasn't made for it.

@rchl
Copy link

rchl commented Sep 16, 2022

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

@hauntsaninja
Copy link
Collaborator

Apologies that your time got wasted on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests