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

Prefer stdlib modules over same-named modules on sys.path #2223

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jacobtylerwalls
Copy link
Member

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Prefer stdlib modules over same-named modules on sys.path

For example:
import copy now finds copy instead of copy.py.

This worked correctly before in at least some cases if there was a (more?) complete chain of __init__.py files from cwd all the way to the location of the copy.py module.

Closes pylint-dev/pylint#6535

ChangeLog Outdated Show resolved Hide resolved
@jacobtylerwalls jacobtylerwalls force-pushed the same-name-stdlib branch 5 times, most recently from 02f375b to 7a6b60f Compare June 26, 2023 01:21
For example:
`import copy` now finds `copy` instead of `copy.py`.

This worked correctly before in at least some cases if there
was a (more?) complete chain of __init__.py files from cwd
all the way to the location of the `copy.py` module.

Closes pylint-dev/pylint#6535
@codecov
Copy link

codecov bot commented Jun 26, 2023

Codecov Report

Merging #2223 (849f68e) into main (aa7916a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2223   +/-   ##
=======================================
  Coverage   92.85%   92.85%           
=======================================
  Files          94       94           
  Lines       11053    11055    +2     
=======================================
+ Hits        10263    10265    +2     
  Misses        790      790           
Flag Coverage Ξ”
linux 92.66% <100.00%> (+<0.01%) ⬆️
pypy 90.99% <100.00%> (+<0.01%) ⬆️
windows 92.44% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Ξ”
astroid/interpreter/_import/spec.py 97.43% <100.00%> (+0.02%) ⬆️

@@ -157,6 +157,19 @@ def find_module(
location=getattr(spec.loader_state, "filename", None),
type=ModuleType.PY_FROZEN,
)
if (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this clash with namespace modules?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you flesh this out a little more for me? (Sorry!)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❯ tree .
.
β”œβ”€β”€ argparse.py
└── test.py

1 directory, 2 files
# test.py

import argparse

argparse.i_dont_exist()
# argparse.py

def i_dont_exist():
    print("1")
❯ python test.py
1

Won't this implementation default to stdlib.argparse instead of the local argparse?

This comment was marked as outdated.

Copy link
Member Author

@jacobtylerwalls jacobtylerwalls Jun 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, in the comment I just hid I hadn't run the test case against the correct copy of astroid. More soon.

@jacobtylerwalls jacobtylerwalls marked this pull request as draft June 27, 2023 23:09
@jacobtylerwalls jacobtylerwalls removed the pylint-tested PRs that don't cause major regressions with pylint label Jul 1, 2023
@jacobtylerwalls jacobtylerwalls modified the milestones: 3.0.0a6, 3.0.0b0 Jul 4, 2023
@jacobtylerwalls jacobtylerwalls removed this from the 3.0.0b0 milestone Aug 5, 2023
@jacobtylerwalls jacobtylerwalls added the Needs take over πŸ›ŽοΈ Orignal implementer went away but the code could be salvaged. label Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug πŸͺ³ Needs take over πŸ›ŽοΈ Orignal implementer went away but the code could be salvaged. Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pylint does not follow python import order : Builtin modules are imported before custom modules
3 participants