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
Better stdlib support for Path objects using .path attribute #66760
Comments
pathlib is really nice, but currently it's rather inconvenient to use due to the lack of support in other parts of the stdlib for Path objects. For historical reasons, everything accepts string paths, but few places accept Paths. As an example: configparser.ConfigParser.read() but there are lots of others. I'm opening this bug to start a conversation about better support for Path objects in the stdlib. Against all hope, I wish there was a simple way to extend the compatibility, but I don't like having to sprinkle |
Since we're unlikely to ever change all the places, I'd say it's better to be consistent. I'd rather write str(path) all over the place than having to look up in the docs each time if that specific API happens to support passing Paths directly. However, Antoine has been positive towards more utility methods on Path objects. (Not that ConfigParser read() would fall in that category :) |
On Oct 06, 2014, at 03:43 PM, Georg Brandl wrote:
Have you tried to write a large-ish application using path objects? |
I was about to suggest deriving your own Path class from Path and str, but got a base class layout conflict because Path objects define lots of __slots__ :( |
That would be a rather horrible solution. |
I know :) |
As for adding convenience methods to Path objects, yes, I'm quite open to that. Best is to open an issue when you have a specific idea (and a patch :-)). |
I think I'm missing something here because the idea of doing |
I agree with Brett. Making old functions support I'll add to that the functions in the |
|
Fine, so you add an |
We're looking forward to trivial patches that enable Path handling consistently throughout the stdlib.
Easier said than done in C modules. |
Georg: You're right, I forgot about C modules which make this not as trivial as I thought. As for Python modules: If there's general agreement that this is a feature we want, I'll be happy to make a patch for the |
Well, if you use an isinstance check you privilege the stdlib Path over any other pathlike implementation. Since it *is* in the stdlib, this isn't an automatic reason for rejection, but it does have a bit of a code smell to it. Why should everything that deals with path strings have to have intimate knowledge of Path objects? I originally wrote here "Maybe we need a __path__ magic method" as a half-joke, but rereading the issue I see that this has in fact been proposed seriously, and referenced by Antoine (the pathlib author). I'm -1 on just sprinkling support for Path throughout the stdlib. Do it in a universally applicable fashion or don't do it at all, IMO. |
How about doing it per module? |
Please be conservative with adding methods to Path. FWIW, my own experiences with pathlib have shown that utility methods have the disadvantage of duplicating well-established Python idioms. This reduces code readability in my opinion. While it is desirable that the rather inconvenient os.path may be superseded by pathlib in the long run, utility methods like Path.glob, Path.open, Path.mkdir, and Path.readtext have convenient stdlib counterparts with long tradition, and using the methods seems disruptive and confusing to me. This is a further argument for having a path protocol instead IMO. |
Random idea: what if pathlib.Path defined a .path attribute that was a plain string? Then you could write p.path instead of str(p), and "if hasattr(p, 'path'): p = p.path". This would be the new protocol. Advantage is also that DirEntry (returned by the new os.scandir()) already supports it. :-) |
I thought about it some more, and personally I'd prefer each function to do |
|
Here the aim is really to distinguish path-like objects from other objects, not to accept arbitrary strings. A ".path" attribute sounds like the wrong name, though: a Path has a path? And the Path's path is not a Path? Ouch :-) |
I think it's actually very reasonable for a Path to have a path attribute The shortest idiom in libraries wanting to support this would be path = gettattr(arg, 'path', arg) This extracts the path attribute from a DirEntry or Path object, and |
Would |
Only if we changed DirEntry to support that too. But it's a kind of |
Personally I thought the name |
In any case I don't think "location" is any better ;-) If "path" fits other people then good. |
OK, I'll add 'path' to unblock changes to the stdlib (but I won't close |
New changeset 7e9605697dfc by Guido van Rossum in branch '3.4': New changeset 9c49c417a68a by Guido van Rossum in branch '3.5': New changeset d5f96a5da219 by Guido van Rossum in branch 'default': |
No docs? ;) |
New changeset 2e3c31ab586a by Guido van Rossum in branch '3.4': New changeset 408f8b255b56 by Guido van Rossum in branch '3.5': New changeset 759b2cecc289 by Guido van Rossum in branch '3.4': New changeset 1a6b485e717f by Guido van Rossum in branch '3.5': New changeset eab349b5c6d7 by Guido van Rossum in branch '3.5': New changeset 97ab0ccac893 by Guido van Rossum in branch 'default': |
So, I added docs, mentioning the getattr(arg, 'path', arg) idiom, and (for 3.5 and 3.6) also cross-referencing with DirEntry. I'm not sure whether to now close this issue or whether to leave it open to remind people of adding patches using the new idiom to various stdlib modules. Opinions? Also, since pathlib is provisional, I felt okay with adding this to 3.4.5 and 3.5.2. |
We could leave this open as a meta issue and spin off individual issues for any specific module people choose to update to support Path objects, setting those new issues as dependencies here. |
Opened bpo-26027 for adding support in the posix module. Should the path attribute be only string, or other types are acceptable (bytes, file descriptor, and even None if the function accepts None)? |
Let's say that the path attribute should be str or bytes -- this matches |
Here's an alternate idea I thought of now. Maybe
What I like about this is that it avoids having the path argument be a string. I don't like misnomers :) I'd understand though if people won't like this suggestion since it introduces another step, of turning the path back into a string. |
I appreciate the thought, Ram, but I'm not a fan. |
Using 'path' for this field is not a misnomer (example: DictEntry uses 'path' for the same field). See issue bpo-26027 for a follow-up idea. |
DirEntry.path can be bytes. |
You can't win 'em all... :-) |
New changeset d0c8b2c1544e by Serhiy Storchaka in branch '3.5': New changeset 719c11b6b6ff by Serhiy Storchaka in branch 'default': New changeset 7197809a7428 by Serhiy Storchaka in branch '2.7': |
Sorry, these changesets were related to bpo-26200. |
Reopening as we need to rename the path attribute to __fspath__ once Brett's PEP is accepted (which will be soon). https://github.com/brettcannon/path-pep/blob/master/pep-0NNN.rst The 3.4 and 3.5 versions of this should probably just be reversed before their releases (early June). |
PEP-519 is accepted now. We need to revert the commits from http://bugs.python.org/issue22570#msg257634 |
And those from http://bugs.python.org/issue22570#msg257632 (these are the actual code -- the others were the docs). |
Done. The revs are 90e58a77d386, 97198545e6c3, ade839421b8f. |
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: