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

Be more aggressive about site-packages in MYPYPATH #7458

Merged
merged 3 commits into from Sep 4, 2019

Conversation

@ilevkivskyi
Copy link
Collaborator

commented Sep 4, 2019

Fixes dropbox/sqlalchemy-stubs#77

Alternatively (if this looks too aggressive), we can at least remove os.path.sep when performing the check.

@ilevkivskyi ilevkivskyi requested review from msullivan and JukkaL Sep 4, 2019
@@ -493,8 +493,10 @@ def compute_search_paths(sources: List[BuildSource],
egg_dirs, site_packages = get_site_packages_dirs(options.python_executable)
for site_dir in site_packages:
assert site_dir not in lib_path
if site_dir in mypypath:
if any(p.startswith(site_dir) for p in mypypath):

This comment has been minimized.

Copy link
@JukkaL

JukkaL Sep 4, 2019

Collaborator

If site_dir is /foo/bar, should we allow things like /foo/bar2 in the mypy path? I.e., maybe we should require that there is a path separator after the site_dir.

This comment has been minimized.

Copy link
@ilevkivskyi

ilevkivskyi Sep 4, 2019

Author Collaborator

OK, makes sense.

This comment has been minimized.

Copy link
@ilevkivskyi

ilevkivskyi Sep 4, 2019

Author Collaborator

The new version disables /foo/bar, /foo/bar/, /foo/bar/baz, but allows /foo/bar2.

Ivan Levkivskyi
@JukkaL
JukkaL approved these changes Sep 4, 2019
Copy link
Collaborator

left a comment

Thanks for the update! This should help with one of more confusing error modes users can get into. One more comment about Windows.

@@ -493,8 +493,10 @@ def compute_search_paths(sources: List[BuildSource],
egg_dirs, site_packages = get_site_packages_dirs(options.python_executable)
for site_dir in site_packages:
assert site_dir not in lib_path
if site_dir in mypypath:
if site_dir in mypypath or any(p.startswith(site_dir + os.path.sep) for p in mypypath):

This comment has been minimized.

Copy link
@JukkaL

JukkaL Sep 4, 2019

Collaborator

Hmm on Windows we may want to also support os.path.altsep.

Ivan Levkivskyi
@ilevkivskyi ilevkivskyi merged commit 6338078 into python:master Sep 4, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ilevkivskyi ilevkivskyi deleted the ilevkivskyi:fix-site-packages branch Sep 4, 2019
g-k added a commit to mozilla-services/find-package-rugaru that referenced this pull request Oct 2, 2019
For our config 0.730 doesn't seem to resolve site-packages well

refs: python/mypy#7458
g-k added a commit to mozilla-services/find-package-rugaru that referenced this pull request Oct 2, 2019
For our config 0.730 doesn't seem to resolve site-packages well

refs: python/mypy#7458
g-k added a commit to mozilla-services/find-package-rugaru that referenced this pull request Oct 2, 2019
For our config 0.730 doesn't seem to resolve site-packages well

refs: python/mypy#7458
g-k added a commit to mozilla-services/find-package-rugaru that referenced this pull request Oct 4, 2019
For our config 0.730 doesn't seem to resolve site-packages well

refs: python/mypy#7458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.