This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: pathlib.Path methods can raise NotImplementedError
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.11
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: AlexWaygood, barneygale, eric.araujo, eryksun, pitrou, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2022-02-12 19:35 by barneygale, last changed 2022-04-11 14:59 by admin.

Pull Requests
URL Status Linked Edit
PR 31338 open barneygale, 2022-02-14 19:55
PR 31339 open barneygale, 2022-02-14 19:55
PR 31340 open barneygale, 2022-02-14 19:55
PR 31341 open barneygale, 2022-02-14 19:55
Messages (11)
msg413142 - (view) Author: Barney Gale (barneygale) * Date: 2022-02-12 19:35
The docs for NotImplementedError say:

> In user defined base classes, abstract methods should raise this exception when they require derived classes to override the method, or while the class is being developed to indicate that the real implementation still needs to be added.

pathlib's use of NotImplementedError appears to be more broad. It can be raised in the following circumstances:

1. When attempting to construct a WindowsPath from a non-Windows system, and vice-versa. This is the only case where NotImplementedError is mentioned in the pathlib docs (in a repl example)
2. In glob() and rglob() when an absolute path is supplied as a pattern
3. In owner() if the pwd module isn't available
4. In group() if the grp module isn't available
5. In readlink() if os.readlink() isn't available
6. In symlink_to() if os.symlink() isn't available
7. In hardlink_to() if os.hardlink() isn't available
8. In WindowsPath.is_mount(), unconditionally

I suspect there are better choices for exception types in all these cases.
msg413143 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2022-02-12 20:24
Here's my two cents, as a non-expert when it comes to pathlib:

I'm not really sure why `is_mount()` exists on WindowsPath objects, given that it unconditionally raises `NotImplementedError` on WindowsPath objects -- that seems *very* strange to me. It seems to me like it should probably be a method on the `PosixPath` class rather than on the `Path` class.

The other methods that raise NotImplementedError don't seem as egregious, because none of them *unconditionally* raise NotImplementedError. We could debate whether some of them would raise different errors in an ideal world, but changing them now might have implications for backwards compatibility, and it doesn't seem like a *major* issue to me.
msg413144 - (view) Author: Alex Waygood (AlexWaygood) * (Python triager) Date: 2022-02-12 20:28
I suppose it might also be worth considering moving `owner()` and `group()` to PosixPath. In practice, these unconditionally raise NotImplementedError on Windows, since the pwd and grp modules are not available on Windows. So, in an ideal world, they probably wouldn't exist on the common base class.

Again, though, that needs to be weighed against backwards-compatibility considerations.
msg413263 - (view) Author: Barney Gale (barneygale) * Date: 2022-02-14 21:01
Thanks very much Alex. I've added some PRs:

PR 31338 addresses owner(), group() and is_mount(). It moves those methods to PosixPath, and adds stubs in WindowsPath that raise deprecation warnings. I agree with your analysis that, for static typing purposes, these methods shouldn't even exist on WindowsPath!

PR 31339 addresses readlink(), symlink_to() and hardlink_to(). In this case I'm working towards making those methods unavailable if os.readlink(), symlink() and link() are unavailable. Not totally sold on this - thoughts?

PR 31340 addresses glob() and rglob(), switching the exception type to ValueError. I think this is a legitimate bugfix with minimal adverse effects.

PR 31341 addresses the Path constructor. This is a backwards incompatible change, and /probably/ not worth doing. I add it for completeness sake, as these four PRs cover all cases where pathlib raises NotImplementedError.
msg413276 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2022-02-15 04:25
WindowsPath.is_mount() should call ntpath.ismount(). This function needs a significant redesign, but it's fine to use it as long as it's documented that is_mount() is equivalent to os.path.ismount().

Since the owner() and group() methods return names instead of numeric IDs, technically we could implement them in WindowsPath. That said, in Windows, st_mode and chmod() are unrelated to a file's owner and group, plus object ownership is fundamentally different from POSIX. Unless chown() is also implemented, I don't think we would gain much from knowing the owner and group, other than making it easier to display them.
msg413298 - (view) Author: Barney Gale (barneygale) * Date: 2022-02-15 17:17
I'm planning to learn more heavily on posixpath + ntpath in pathlib once bpo-44136 is done. I think that would be a good time to introduce is_mount() support on Windows.
msg413308 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2022-02-16 01:49
> I'm planning to learn more heavily on posixpath + ntpath in 
> pathlib once bpo-44136 is done. I think that would be a good 
> time to introduce is_mount() support on Windows.

In the long run, it would be better to migrate the implementations in the other direction. Rewrite genericpath, ntpath, posixpath, and parts of shutil to use PurePath and Path objects. Using path objects instead of generic strings should improve the implementation of os.path and shutil, in addition to ensuring consistency with pathlib. However, that's a long-term goal that doesn't preclude using os.path and shutil as needed now, such as relying on os.path.ismount().
msg413506 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2022-02-18 19:48
I think pathlib should be raising TypeError, ValueError, or something else as appropriate.  Or not raising in situations Eryk indicated.  x/0 *could* be +- float('inf'), but we don't raise NotImplementedError for it.
msg413627 - (view) Author: Barney Gale (barneygale) * Date: 2022-02-21 02:08
Thanks both. I've adjusted PR 31338 to leave is_mount() unchanged. I've opened a new pull request that implements is_mount() on Windows using ntpath.ismount(): PR 31458
msg413644 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-02-21 10:33
> In the long run, it would be better to migrate the implementations in the other direction. Rewrite genericpath, ntpath, posixpath, and parts of shutil to use PurePath and Path objects.

pathlib does not allow to distinguish "path" from "path/".
msg413647 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2022-02-21 11:42
> pathlib does not allow to distinguish "path" from "path/".

os.path.normpath() and os.path.abspath() don't retain a trailing slash -- or a leading dot component for that matter. Are you referring to os.path.join()? For example:

    >>> os.path.join('./spam', 'eggs/')
    './spam/eggs/'
    >>> os.path.normpath('./spam/eggs/')
    'spam/eggs'
    >>> PurePath('./spam') / PurePath('eggs/')
    PurePosixPath('spam/eggs')

A leading dot component is significant in a context that searches a set of paths -- usually PATH. A trailing slash is significant in a context that has to distinguish a device or file path from a directory path, of which there are several cases in Windows.

I think it's a deficiency in pathlib that it lacks a way to require conservative normalization in these cases. Path and PurePath objects could gain a keyword-only parameter, and internal attribute if needed, that enables a more conservative normalization.
History
Date User Action Args
2022-04-11 14:59:56adminsetgithub: 90889
2022-02-21 11:42:45eryksunsetmessages: + msg413647
2022-02-21 10:33:08serhiy.storchakasetmessages: + msg413644
2022-02-21 02:08:14barneygalesetmessages: + msg413627
2022-02-18 19:48:41terry.reedysetnosy: + terry.reedy
messages: + msg413506
2022-02-16 01:49:24eryksunsetmessages: + msg413308
2022-02-15 17:17:22barneygalesetmessages: + msg413298
2022-02-15 04:25:32eryksunsetnosy: + eryksun
messages: + msg413276
2022-02-14 21:01:14barneygalesetmessages: + msg413263
2022-02-14 19:55:37barneygalesetpull_requests: + pull_request29492
2022-02-14 19:55:33barneygalesetpull_requests: + pull_request29491
2022-02-14 19:55:29barneygalesetpull_requests: + pull_request29490
2022-02-14 19:55:14barneygalesetkeywords: + patch
stage: patch review
pull_requests: + pull_request29489
2022-02-13 18:12:42eric.araujosetnosy: + pitrou, eric.araujo, serhiy.storchaka
2022-02-12 20:28:10AlexWaygoodsetmessages: + msg413144
2022-02-12 20:24:53AlexWaygoodsetnosy: + AlexWaygood
messages: + msg413143
2022-02-12 20:15:13AlexWaygoodsetkeywords: - patch
stage: patch review -> (no value)
2022-02-12 20:14:54AlexWaygoodsetpull_requests: - pull_request29461
2022-02-12 19:38:00barneygalesetkeywords: + patch
stage: patch review
pull_requests: + pull_request29461
2022-02-12 19:35:13barneygalecreate