Skip to content

bpo-32642: Allow for PathLike objects in sys.path #22000

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.

Sign up for GitHub

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

Jump to bottom
Open
sroet wants to merge 6 commits into
base: main
Choose a base branch
from
Open

bpo-32642: Allow for PathLike objects in sys.path #22000

sroet wants to merge 6 commits into from

Conversation

@sroet
Copy link

@sroet sroet commented Aug 29, 2020 •
edited by bedevere-bot
Loading

This is similar to what was done in #5691, but that one seems abandoned.

It was mentioned there a test needed to be included.
My current idea of a test involves making files outside of the running directory to be added to sys.path and then imported.
I would like some guidance on what the best/common practices are for that in this code-base.

test that shows the issue (taken from issue32642):
t.py

import pathlib                                                                  
import sys                                                                      
                                                                                
sys.path.append(pathlib.Path('foo'))                                            
                                                                                
# uncomment next line for this script to pass on master                                
# sys.path.append('foo')                                                        
                                                                                
import s

in foo/s.py

print(123)

on master this gives:

Traceback (most recent call last):
  File "t.py", line 9, in <module>
    import s
ModuleNotFoundError: No module named 's'

With this implementation:

123

https://bugs.python.org/issue32642

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

the-knights-who-say-ni commented Aug 29, 2020

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).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@sroet

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

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

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

ZackerySpytz
ZackerySpytz reviewed Jan 10, 2021
View reviewed changes
Copy link
Contributor

@ZackerySpytz ZackerySpytz left a comment

Choose a reason for hiding this comment

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

Please add tests.

I believe the documentation for sys.path should be updated.

Lib/importlib/_bootstrap_external.py
Comment on lines +1334 to +1335
if not (isinstance(entry, (str, bytes)) or
hasattr(entry, '__fspath__')):
Copy link
Contributor

@ZackerySpytz ZackerySpytz Jan 10, 2021

Choose a reason for hiding this comment

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

It seems that this change doesn't take the comment on the old PR into account.

Copy link
Author

@sroet sroet Jan 13, 2021

Choose a reason for hiding this comment

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

If you are talking about this comment.

I interpreted that discussion as " str and bytes could also be fed through _os.fspath ", which is what this PR does.

Do you have a different interpretation of that comment?

Misc/NEWS.d/next/Library/2020-08-31-10-29-46.bpo-32642._tSFZV.rst
@@ -0,0 +1 @@
Add compatibility for path-like objects on sys.path. Patch by sroet
Copy link
Contributor

@ZackerySpytz ZackerySpytz Jan 10, 2021

Choose a reason for hiding this comment

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

Markup could be used here.

:term:`path-like objects <path-like object>`
:data:`sys.path`

Please end sentences with periods.

@sroet
Copy link
Author

sroet commented Jan 13, 2021

Please add tests.

As stated in the original comment:

My current idea of a test involves making files outside of the running directory to be added to sys.path and then imported.
I would like some guidance on what the best/common practices are for that in this code-base.

Would you have any guidance on where the test file should live and how adding "files outside of the running directory" is commonly handled?

@cjerdonek cjerdonek mentioned this pull request May 6, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ZackerySpytz ZackerySpytz ZackerySpytz left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sroet @the-knights-who-say-ni @ZackerySpytz @ezio-melotti @bedevere-bot