-
-
Notifications
You must be signed in to change notification settings - Fork 30k
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
add support for path-like objects in sys.path #76823
Comments
|
This issue is to suggest enhancing sys.path to recognize path-like objects, per PEP-519. I recently ran into an issue where a path was getting added to sys.path, but the corresponding imports weren't working as they should, even though sys.path showed the path as being present. Eventually I tracked this down to the path being a pathlib.Path object rather than a string. This wasn't obvious because Path objects appear as strings in normal debug output, etc. The sys.path docs currently say this:
|
|
This looks a lot like https://bugs.python.org/issue32446, I'd like to tackle this, if we are going through with it. |
|
what file(s) is/are the sys.path code located in? |
If I understand it correctly, sys.path is handled in importlib._bootstrap_external.PathFinder.find_spec(). I can patch PathFinder for handling path-like objects: https://github.com/yan12125/cpython/commit/e3fd473b54cbb368533e651fd896bbc813a43924 Here's an example usage: # t.py
import pathlib
import sys
sys.path.append(pathlib.Path('foo'))
import s
# foo/s.py
print(123) |
|
Thanks for the reply |
|
ok, so I found the PathFinder class you referenced, just making sure, this issue pertains to changing "self.path"'s usage and declaration to be a path-like object instead of the hard coded 'sys', 'path' returns? or is that part of it already?, also since this uses a tuple to keep track of the parent path, is there functionality in the path-like objects to find parent paths? |
|
cpython/Lib/importlib/_bootstrap_external.py Lines 1069 to 1090 in 3c34aad
those are the particular class and lines I'm referring to |
|
I'm unsure how to regenerate the files that interact with the code for sys.path as travisCI states |
|
'make regen-all' should do it (but so will just running 'make -s -j' like On Mon, Mar 5, 2018, 12:43 Jay Yin, <report@bugs.python.org> wrote:
|
|
I've been stuck on "test_poplib" for over 149661 sec now, that's about 41 hours... I don't think this is working correctly, although I'm unsure what test_poplib is doing that has been affected by what I've changed here. |
|
The issue was resolved by updating my version of the rest of the package apparently and remaking the whole thing, must have been some outdated stuff on my end causing the issue |
|
I'm having issues with my local changes for my PR, I'm unsure why my local machine takes a seemingly infinite amount of time on test_poplib, so again I think something to do with my local environment is causing issues again, any help would be appreciated... |
|
the trace for the test_poplib |
|
it seems to me like the issue in my tests is that some SSL thing is failing?, anyone have any experience with this? |
|
Please open a separate issue for the test_poplib issue. |
|
srry I opened another issue bpo-33099 |
|
This seems fine to me and I can't think of any negatives. Can anyone think of why this might be a bad idea? (Messing with how sys.path is used is just so fundamental I'm being paranoid. :) |
|
I’m not an import expert but would have misgivings about having fancy types in sys.path too! It seems so fundamental, and used from C and Python, with a simple interface of a direct list (plus importers and finders etc), that I would understand it pure strings were required and not Paths (and not all path-likes). |
|
I'm in support of adding Path support for sys.path, but I also agree with Eric, there are innumerable consumers of sys.path beyond importlib. and since pathlib.Path isn't a str, it would likely introduce incompatibility. On the other hand, users introducing Path objects to sys.path could be warned that although importlib supports Path objects, other consumers may not, and that support for it in importlib isn't endorsement of the use of those types and the consequences aren't necessarily supported. As an aside, it's too bad a Path object couldn't have been a str subclass (as it is for path, which would have made problems like this one much safer to solve. |
|
Are there any problems with converting a Path to string before adding it to sys.path? You do this one time, and any users of sys.path will not need to bother about conversion. It is better even for performance. Otherwise we will need to revise and update every code that uses sys.path, and many bugs will left in third-party code for years. For example, in unittest the code if not top_level_dir in sys.path:
sys.path.insert(0, top_level_dir)should be replaced with more cumbersome for path in sys.path:
if os.fsdecode(path) == top_level_dir:
break
else:
sys.path.insert(0, top_level_dir)In multiprocessing the code sys_path=sys.path.copy()
try:
i = sys_path.index('')
except ValueError:
pass
else:
sys_path[i] = process.ORIGINAL_DIRshould be replaced with sys_path=sys.path.copy()
for i, path in enumerate(sys_path):
if os.fsdecode(path) == '':
sys_path[i] = process.ORIGINAL_DIR
breakIt is just two examples. I did not review the whole stdlib, and there should be more third-party code. |
|
You've got a point. What do you feel about sub-classing list for sys.path and converting path-like objects upon setting items / reading? |
|
The import system is already complex, so I think the hesitation about allowing arbitrary Path-like objects is warranted. (For example: are importlib's caching semantics really valid for *arbitrary* path-like objects? An object can be path-like without being immutable) Coercion on input (as Noam suggests) would have a much lower risk of unwanted side effects, as any dynamic behaviour would be resolved at insertion time. |
|
So I've got in mind a PyListObject subclass with calls to PyOS_FSPath before insert, append, extend and __getitem__. But I feel like this is an overkill; also, extend function might be trickier to implement. |
|
I think you are trying to solve a wrong problem.
How is it? >>> pathlib.Path('/usr/lib')
PosixPath('/usr/lib')
>>> [pathlib.Path('/usr/lib')]
[PosixPath('/usr/lib')]I think the problem is with the debug output. Always use repr() for it, otherwise you will trick yourself.
It would be easier to catch if it was never working with non-string. I think we should deprecate any non-string elements in sys.path. As I shown above some code only works correctly with strings anyway. |
|
pkgutil just skips non-string elements in sys.path. for dir in search_path:
if not isinstance(dir, str):
continue |
|
that's why pre-insert conversion was suggested. |
|
On 26.03.2022 08:59, Nick Coghlan wrote:
This is not only about the import system. Lots of Python code out there https://docs.python.org/3/library/sys.html#sys.path Conversion to strings sounds like a good way to get the best out of I'm just curious on how this would work. You'd like have to create a |
The sys module doesn't prevent rebinding sys.path. Most code I think is careful to update sys.path. But surely some code replaces it with a new list instead of updating via insert(), append(), extend(), or updating a slice such as |
|
Why use a subclass of PyListObject when you could use composition? Create a singleton SysPath object containing a list. Add pass through methods for all of the List behavior you want. Any methods that modify the list by adding an item can convert the item to an str. This might make sys.path much easier to understand and maintain. Letting people completely replace the whole thing would seem to make it very fragile. |
|
This change requires a discussion on python-dev, maybe even a PEP. Closing this ticket for now, please reopen if a discussion reaches positive agreement on this. |


Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: