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

bpo-32642: add support for path-like objects in sys.path #32118

Closed

Conversation

noamcohen97
Copy link
Contributor

@noamcohen97 noamcohen97 commented Mar 25, 2022

The sys.path did only accept str or bytes object. This change
makes os.PathLike object to be accepted as well.

https://bugs.python.org/issue32642

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@noamcohen97

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@MaxwellDupre
Copy link
Contributor

Could you add a test please? The existing test doesn't exercise the code changes.

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Jun 10, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

noamcohen97 and others added 2 commits July 19, 2022 08:38
The sys.path did only accept `str` or `bytes` object. This change
makes `os.PathLike` object to be accepted as well.
@noamcohen97 noamcohen97 force-pushed the sys-path-support-path-like-objects branch from 194b978 to a8c5f54 Compare July 19, 2022 05:38
@@ -24,13 +25,12 @@ def remove_modules(self):
del sys.modules[module_name]

def setUp(self):
self.test_dir = tempfile.mkdtemp()
self.test_dir = pathlib.Path(tempfile.mkdtemp())
Copy link
Contributor

Choose a reason for hiding this comment

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

this removes the test for str on sys.path

@@ -1475,7 +1475,10 @@ def _get_spec(cls, fullname, path, target=None):
# the list of paths that will become its __path__
namespace_path = []
for entry in path:
if not isinstance(entry, str):
try:
# bytes, str and path-like objects will pass fspath
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to add a test for bytes on sys.path, and test it with the other import systems. Note these tests fail with a BytesWarning

Copy link
Contributor

Choose a reason for hiding this comment

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

@graingert
Copy link
Contributor

cc @jaraco re: python/importlib_metadata#372

@graingert
Copy link
Contributor

graingert commented Jul 19, 2022

@noamcohen97 the issue for this was closed - #76823 (comment) you should raise the request on discuss https://discuss.python.org/c/ideas/6

@erlend-aasland erlend-aasland added the pending The issue will be closed if no feedback is provided label Jul 19, 2022
@erlend-aasland
Copy link
Contributor

@noamcohen97 the issue for this was closed - [#76823 (comment)] [...]

Suggesting then to close this PR.

@erlend-aasland erlend-aasland removed the pending The issue will be closed if no feedback is provided label Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants