Skip to content

Fix error message for ntpath.commonpath #117381

Closed
Closed
Fix error message for ntpath.commonpath#117381
Labels
OS-windowstype-bugAn unexpected behavior, bug, or error

Description

@nineteendo
nineteendo
opened on Mar 29, 2024

Bug report

Bug description:

>>> import ntpath
>>> ntpath.commonpath(["/foo", "foo"])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<frozen ntpath>", line 808, in commonpath
ValueError: Can't mix absolute and relative paths

/foo is not an absolute path, but a rooted path, which can't be mixed with not-rooted paths.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Windows

Linked PRs

Activity

  1. nineteendo
    added
    type-bugAn unexpected behavior, bug, or error
    on Mar 29, 2024
  2. aisk
    added
    OS-windows
    on Mar 30, 2024

  3. aisk commented on Mar 30, 2024

    @aisk
    aisk
    on Mar 30, 2024
    Member

    I did some googling and found that Microsoft has a document which describes "relative rooted path": https://learn.microsoft.com/en-us/dotnet/api/system.io.path.ispathrooted?view=net-8.0#remarks

    A relative rooted path specifies a drive, but its fully qualified path is resolved against the current directory.

    So I think the relative rooted path should be something like C:foo (please notice that it's not C:/foo), but /foo still is an absolute path. And ntpath.isabs('/foo') returns True, so I think it's an absolute path.

  4. nineteendo commented on Mar 30, 2024

    @nineteendo
    nineteendo
    on Mar 30, 2024
    ContributorAuthor

    ntpath.isabs('/foo') returns True

    That was recently changed:

    The error message could also be "Can't mix rooted and not-rooted paths" as /foo is rooted and foo is not.

  5. zooba commented on Apr 2, 2024

    @zooba
    zooba
    on Apr 2, 2024
    Member

    How does this change make life better or easier for those encountering this message?

    At first glance, it appears to be getting into overly precise terminology that doesn't help educate or correct the caller.

  6. nineteendo commented on Apr 2, 2024

    @nineteendo
    nineteendo
    on Apr 2, 2024
    ContributorAuthor

    The error message states: "Can't mix absolute and relative paths".
    But /foo & foo are both relative according to ntpath.isabs():

    Path Rooted? Absolute?
    c:/foo Yes Yes
    c:foo Yes No
    /foo Yes No
    foo No No

    So, this error message is factually incorrect.
    But /foo is rooted and foo is not, so that's what the new error message is in that case.
    On a side note, it could be useful to add os.path.isrooted.

  7. serhiy-storchaka
    added a commit that references this issue on Apr 3, 2024

    gh-117381: Improve error messages for ntpath.commonpath() (GH-117382)

    2ec6bb4
  8. serhiy-storchaka
    closed this as completedon Apr 3, 2024
  9. diegorusso
    added a commit that references this issue on Apr 17, 2024

    pythongh-117381: Improve error messages for ntpath.commonpath() (pyth…

    6463fe5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    OS-windowstype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions