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

sitecustomize/full sys.path support #5701

Closed
llchan opened this issue Oct 1, 2018 · 12 comments · Fixed by #11143
Closed

sitecustomize/full sys.path support #5701

llchan opened this issue Oct 1, 2018 · 12 comments · Fixed by #11143

Comments

@llchan
Copy link
Contributor

llchan commented Oct 1, 2018

Looking at sitepkgs.py, the current implementation only looks for PEP 561 packages in site.usersitepackages() and site.getsitepackages(). My environment has a sitecustomize.py that adds additional paths to sys.path based on some in-house env vars (not really my choice, I inherited this setup). Could we add a mypy flag to indicate that it should use the full sys.path? I don't really see a harm in including PYTHONPATH too.

On a related note, shouldnt user site packages have precedence over system site packages?

@emmatyping
Copy link
Collaborator

We should probably support this. I am a little concerned that it will be hard to figure out when packages passed via command line are on the path, but I will have to think more about this.

I also want to refactor sitepkgs.py to be a more general script used to get all the information we need from an external Python executable (version, location, etc).

@gvanrossum
Copy link
Member

As long as we filter by the presence of py.typed or the directory suffix -stubs I don't see a problem with searching all of $PYTHONPATH. If we do that, do we need a change to PEP 561?

An alternative could be to search for these on all of $MYPYPATH. (Though it's less powerful because it doesn't automatically vary with --python-version.)

@llchan
Copy link
Contributor Author

llchan commented Oct 1, 2018

Thanks for the consideration. Just to make sure I'm explicit about it, it's not just PYTHONPATH that I'd like to see, but the full site.py logic that includes sitecustomize.py or maybe usercustomize.py.

I would probably go with the "filter things out from sys.path" rather than the "recreate the logic in site.py" approach, so that future changes in site.py will likely work without replicating the logic in mypy, but there may be nuances I'm not considering.

@emmatyping
Copy link
Collaborator

emmatyping commented Oct 12, 2018

As long as we filter by the presence of py.typed or the directory suffix -stubs I don't see a problem with searching all of $PYTHONPATH.

I don't think this is the correct thing to do here. The directories added to the sys.path are not "installed" really. I was concerned that it would problematic to remove the installed packages and current directories from the sys.path that we scan, but thinking about an implementation more, I don't think this will be as much of an issue. I'll give the implementation a try and if I find it isn't working out, we can always re-evaluate.

@llchan
Copy link
Contributor Author

llchan commented Aug 12, 2019

Circling back: if I modify mypy.sitepkgs.getsitepackages to return sys.path, this does the right thing for me, searching for PEP 561 packages on the sys.path that would be used if that python execuable were run. Is there any reason not to do that? If it seems reasonable I'll send up a PR.

@emmatyping
Copy link
Collaborator

I'm not sure if this is safe or not, I'll have to think about that. I'm very busy this week so I won't be able to review anything until Saturday at the earliest, but feel free to open a PR now or then if you want to.

@jraygauthier
Copy link

Any progress here? This impact nix users which uses sitecustomize to create reproducible development environments. Mypy is actually barely usable in this context. We're plagged with these unexpected error: Cannot find module named 'my_package' for our PEP 561 libraries which do expose the py.typed file. Workaround is the add the library to MYPYPATH. However, this is unintuitive and contrary to documentation and it was really tough to find why mypy wasn't finding the packages even after reading the full doc. One clearly expects that if a package can be imported by the python interpreter, it should be possible for the matching mypy exe to import its types.

@emmatyping
Copy link
Collaborator

Ok, it seems there is significant need for this, bumping to priority high.

@jaraco
Copy link
Member

jaraco commented Dec 27, 2020

I took a stab at testing the proposed approach (just return sys.path in getsitepackages):

diff --git a/mypy/sitepkgs.py b/mypy/sitepkgs.py
index 2a13e4b2..56444006 100644
--- a/mypy/sitepkgs.py
+++ b/mypy/sitepkgs.py
@@ -7,8 +7,9 @@ library found in Python 2. This file is run each mypy run, so it should be kept
 possible.
 """
 
+import sys
+
 if __name__ == '__main__':
-    import sys
     sys.path = sys.path[1:]  # we don't want to pick up mypy.types
 
 from distutils.sysconfig import get_python_lib
@@ -20,6 +21,11 @@ if MYPY:
 
 
 def getsitepackages():
+    # type: () -> List[str]
+    return _get_site_packages() + sys.path
+
+
+def _get_site_packages():
     # type: () -> List[str]
     if hasattr(site, 'getusersitepackages') and hasattr(site, 'getsitepackages'):
         user_dir = site.getusersitepackages()

But this led to early failures in basic tests (among others):

_________________________________________________________ testWritesCacheErrors _________________________________________________________
[gw1] darwin -- Python 3.9.0 /Users/jaraco/code/public/python/mypy/.tox/python/bin/python
data: /Users/jaraco/code/public/python/mypy/test-data/unit/check-basic.test:345:
/Users/jaraco/code/public/python/mypy/mypy/test/testcheck.py:133: in run_case
    self.run_case_once(testcase)
/Users/jaraco/code/public/python/mypy/mypy/test/testcheck.py:228: in run_case_once
    assert_string_arrays_equal(output, a, msg.format(testcase.file, testcase.line))
/Users/jaraco/code/public/python/mypy/mypy/test/helpers.py:116: in assert_string_arrays_equal
    raise AssertionError(msg)
E   AssertionError: Unexpected type checker output (/Users/jaraco/code/public/python/mypy/test-data/unit/check-basic.test, line 345)
--------------------------------------------------------- Captured stderr call ----------------------------------------------------------
Expected:
  tmp/e.py:1: error: Unsupported operand types for + ("int" and "str") (diff)
Actual:
  (empty)

I also tried simply returning sys.path but that showed comparable results:

diff --git a/mypy/sitepkgs.py b/mypy/sitepkgs.py
index 2a13e4b2..744aef9d 100644
--- a/mypy/sitepkgs.py
+++ b/mypy/sitepkgs.py
@@ -7,13 +7,11 @@ library found in Python 2. This file is run each mypy run, so it should be kept
 possible.
 """
 
+import sys
+
 if __name__ == '__main__':
-    import sys
     sys.path = sys.path[1:]  # we don't want to pick up mypy.types
 
-from distutils.sysconfig import get_python_lib
-import site
-
 MYPY = False
 if MYPY:
     from typing import List
@@ -21,11 +19,7 @@ if MYPY:
 
 def getsitepackages():
     # type: () -> List[str]
-    if hasattr(site, 'getusersitepackages') and hasattr(site, 'getsitepackages'):
-        user_dir = site.getusersitepackages()
-        return site.getsitepackages() + [user_dir]
-    else:
-        return [get_python_lib()]
+    return sys.path
 
 
 if __name__ == '__main__':

Is that because the test suite is tuned to the prior assumption and one other tweak would cause the tests to stop failing? Or does this change break some other fundamental assumption?

@ofek
Copy link

ofek commented Jan 12, 2022

The patch provided by @jaraco completely works for me (ofek/hatch-mypyc#2) as a workaround for #10829 when being built by pip

fabianhjr pushed a commit to LibreCybernetics/mypy that referenced this issue Mar 20, 2022
AWhetter added a commit to AWhetter/mypy that referenced this issue Apr 1, 2022
@Apteryks
Copy link

Apteryks commented Apr 5, 2022

Any progress here? This impact nix users which uses sitecustomize to create reproducible development environments. Mypy is actually barely usable in this context.

The same is also true for GNU Guix users, another non-file hierarchy standard distribution.

@Apteryks
Copy link

Apteryks commented Apr 6, 2022

And in my case I can't seem to workaround it easily via the MYPYPATH environment variable as the mypy_path procedure which does the os.getenv('MYPYPATH)` is never called, because of the unsatisfied condition on line 1034 of the mypy/main.py script:

https://github.com/python/mypy/blob/master/mypy/main.py#L1034

ipdb> break mypy/main.py:1034                                                                             
Breakpoint 1 at /home/maxim/src/mypy/mypy/main.py:1034
ipdb> c                                                                                                   
> /home/maxim/src/mypy/mypy/main.py(1034)process_options()
   1032 
   1033     # Set target.
1> 1034     if special_opts.modules + special_opts.packages:
   1035         options.build_type = BuildType.MODULE
   1036         egg_dirs, site_packages = get_site_packages_dirs(options.python_executable)

ipdb> special_opts.modules                                                                                
[]
ipdb> special_opts.packages                                                                               
[]

Apteryks added a commit to Apteryks/mypy that referenced this issue Apr 6, 2022
Fixes python#5701.

* mypy/pyinfo.py (getsitepackages): Use sys.path to discover packages.
* mypy/modulefinder.py (get_site_packages_dirs): Adjust accordingly.
(expand_site_packages, _parse_pth_file, _make_abspath): Remove procedures.
(compute_search_paths): Adjust accordingly.
* mypy/main.py (process_options): Likewise.
AWhetter added a commit to AWhetter/mypy that referenced this issue May 20, 2022
emmatyping pushed a commit that referenced this issue May 30, 2022
Closes #5701

This replaces the old hand crafted search code that was more fragile.
Enzime pushed a commit to Enzime/mypy that referenced this issue May 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants