Skip to content
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

Provide expanduser() on Path objects #63975

Closed
pitrou opened this issue Nov 25, 2013 · 32 comments
Closed

Provide expanduser() on Path objects #63975

pitrou opened this issue Nov 25, 2013 · 32 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@pitrou
Copy link
Member

pitrou commented Nov 25, 2013

BPO 19776
Nosy @pitrou, @vstinner, @PCManticore, @zware, @serhiy-storchaka, @zooba, @vajrasky, @seirl
Files
  • pathlib.patch
  • pathlib1.patch
  • issue19776.patch
  • issue19776_1.patch
  • issue19776_2.patch
  • issue19776_3.patch
  • issue19776_4.patch
  • pathlib_expanduser.patch
  • pathlib_expanduser_2.patch
  • test_pathlib_empty_homedir.patch
  • 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:

    assignee = None
    closed_at = <Date 2015-01-10.08:53:47.847>
    created_at = <Date 2013-11-25.20:02:58.534>
    labels = ['type-feature', 'library']
    title = 'Provide expanduser() on Path objects'
    updated_at = <Date 2015-01-10.08:53:47.846>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2015-01-10.08:53:47.846>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2015-01-10.08:53:47.847>
    closer = 'vstinner'
    components = ['Library (Lib)']
    creation = <Date 2013-11-25.20:02:58.534>
    creator = 'pitrou'
    dependencies = []
    files = ['32919', '32980', '35237', '35666', '35955', '35994', '36102', '36106', '36669', '37613']
    hgrepos = []
    issue_num = 19776
    keywords = ['patch']
    message_count = 32.0
    messages = ['204387', '204640', '204897', '205272', '205280', '208494', '216798', '217209', '217339', '218407', '220422', '220516', '220791', '223090', '223111', '223137', '223461', '223770', '224027', '224037', '224039', '227167', '227174', '233153', '233154', '233220', '233513', '233516', '233518', '233790', '233803', '233805']
    nosy_count = 15.0
    nosy_names = ['pitrou', 'vstinner', 'Arfrever', 'cvrebert', 'neologix', 'Claudiu.Popa', 'python-dev', 'zach.ware', 'serhiy.storchaka', 'steve.dower', 'David.Edelsohn', 'vajrasky', 'antoine.pietri', 'fletom', 'rominf']
    pr_nums = []
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue19776'
    versions = ['Python 3.5']

    @pitrou
    Copy link
    Member Author

    pitrou commented Nov 25, 2013

    Something like expanduser() may be useful on Path objects.

    @pitrou pitrou added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Nov 25, 2013
    @vstinner
    Copy link
    Member

    I wanted to suggest to modify the resolve() method, but '~' is a valid filename... (I tested, and it's annoying because depending on command, sometimes ~ is expanded to /home/haypo sometimes not...)

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Dec 1, 2013

    Hello. Here's a patch for expanduser().

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Dec 5, 2013

    Documentation, please (Doc/library/pathlib.rst)!

    This is the docstring of Path.cwd.

    """Return a new path pointing to the current working directory
    (as returned by os.getcwd()).
    """

    This is the docstring of Path.expanduser.
    """ Return a new path with expanded ~ and ~user constructs.
    """

    Perhaps we need to add "(as returned by os.path.expanduser)"?

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Dec 5, 2013

    Thanks, Vajrasky! Here's the new version of the patch.

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Jan 19, 2014

    Antoine, is this feature still wanted?

    @fletom
    Copy link
    Mannequin

    fletom mannequin commented Apr 18, 2014

    +1

    It's very annoying to have to import expand user from os.path, when pathlib should be a full replacement for that module. Thanks to those working on this!

    @rominf
    Copy link
    Mannequin

    rominf mannequin commented Apr 26, 2014

    I think that absolute method should call expanduser and expandvars (do you plan to include it?) automatically. This should be optional (via default arguments: expanduser=True, expandvars=True.

    @seirl
    Copy link
    Mannequin

    seirl mannequin commented Apr 28, 2014

    I think that absolute method should call expanduser and expandvars (do you plan to include it?) automatically. This should be optional (via default arguments: expanduser=True, expandvars=True.

    I think it shouldn't. (Or shouldn't be set to True by default anyway).
    absolute() method resolves symlinks, and it would make no sense to expand tildes and vars, which are purely a "shell syntax".

    . and .. are real things in the filesystem, ~ is just a notation commonly used (since it's in the SCL spec), but it's not *part* of the path, that's why you can totally have a valid ~ file.

    Making absolute() expand tildes would be illogic, unintuitive and unpythonic.

    (+1 for the .expanduser() patch though, I went here after searching for this feature in the docs).

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented May 13, 2014

    Added a new version of the patch with improvements suggested by Berker Peksag. Thanks for the review!

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Jun 13, 2014

    Antoine, how does my latest patch look?

    @pitrou
    Copy link
    Member Author

    pitrou commented Jun 14, 2014

    Claudiu, sorry for the delay. I'm going to take a look soon!

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Jun 17, 2014

    Attached the new version of the patch with fixes according to Antoine's review.

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Jul 15, 2014

    Since all the comments have been addressed, it would be nice if this gets committed.

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Jul 15, 2014

    This new patch fixes some comments from Serhiy. Thanks for the review!

    @serhiy-storchaka
    Copy link
    Member

    Doc example is still looks confusing. Is your home directory /home?

    There is a question. What should pathlib's expanduser() do in case when user directory can't be determined (or user does not exist)? Perhaps unlike to os.path.expanduser() it should raise an exception (as in many other pathlib's methods).

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Jul 19, 2014

    No, my home directory is actually /root. The attached patch should be clearer now (I hope). Regarding your question, wouldn't checking for this duplicate what os.path.expanduser already does? (Unless checking for the exact same string before returning it.)

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Jul 23, 2014

    There is a question. What should pathlib's expanduser() do in case
    when user directory can't be determined (or user does not exist)?
    Perhaps unlike to os.path.expanduser() it should raise an exception
    (as in many other pathlib's methods).

    Let's see what POSIX says:
    http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_01

    """
    If the system does not recognize the login name, the results are undefined.
    """

    Helpful, isn't it ;-)

    Behaving like os.path.expanduser() would have the advantage of consistency (which is in turn consistent with Unix shells), OTOH, it seems wrong to just return the unexpanded form: for example, if someone calls expanduser('~apache') and there's apache user, returning '~apache' could be dangerous if there was an '~apache' folder in the CWD.

    So I think it would be better to raise an exception.

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Jul 26, 2014

    Here's a version of the patch which raises ValueError when the path can't be expanded. Hopefully, the used approach is good enough.

    @serhiy-storchaka
    Copy link
    Member

    Here is alternative patch which doesn't use os.path.expanduser() and reimplement it's logic. Differences:

    • expanduser() is part of concrete path API. This method access environment.
    • RuntimeError is raised when user home can't be determined.
    • Currently ntpath.expanduser() uses heuristic to expand path with specified username. This works with default homedirs but can return wrong result when homedirs was moved to different locations. WindowsPath.expanduser() also uses heuristic, but more robust. Of course it would be better to get other users homedirs from Windows API, and perhaps we should defer this issue until implementing pwd or like on Windows.
    • Expanded tests.

    Interesting, common idiom to escape tilda in relative path (adding "./" prefix) doesn't work with pathlib, because "." components are ignored.

    @PCManticore
    Copy link
    Mannequin

    PCManticore mannequin commented Jul 26, 2014

    Looks good.

    @pitrou
    Copy link
    Member Author

    pitrou commented Sep 20, 2014

    Serhiy, would you like to update your patch?

    @serhiy-storchaka
    Copy link
    Member

    Sorry. Here is updated patch.

    @pitrou
    Copy link
    Member Author

    pitrou commented Dec 28, 2014

    Steve, would you like to give an opinion on the Windows aspects of this patch? Otherwise I will simply commit it soon.

    @zooba
    Copy link
    Member

    zooba commented Dec 28, 2014

    I thought that USERPROFILE was the preferred environment variable and should be checked before HOME, but I could be wrong. Consistency with the existing expanduser function is more important probably.

    There's almost certainly an API to find an arbitrary user directory, but it may only be available to administrators, so the guess is probably necessary anyway. The directory name doesn't necessarily match the user name either, especially since a lot of Windows users these days authenticate with their email address.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 30, 2014

    New changeset bee697b0fd18 by Antoine Pitrou in branch 'default':
    Issue bpo-19776: Add a expanduser() method on Path objects.
    https://hg.python.org/cpython/rev/bee697b0fd18

    @pitrou pitrou closed this as completed Dec 30, 2014
    @vstinner
    Copy link
    Member

    vstinner commented Jan 6, 2015

    The test fails on the buildbot "PPC64 AIX 3.x". It looks like the home directory of the buildbot slave user is /home/shager/.

    http://buildbot.python.org/all/builders/PPC64%20AIX%203.x/builds/2994/steps/test/logs/stdio

    ======================================================================
    FAIL: test_expanduser (test.test_pathlib.PosixPathTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/test/test_pathlib.py", line 2002, in test_expanduser
        self.assertEqual(p3.expanduser(), P(otherhome) / 'Documents')
    AssertionError: PosixPath('/Documents') != PosixPath('Documents')

    @vstinner vstinner reopened this Jan 6, 2015
    @pitrou
    Copy link
    Member Author

    pitrou commented Jan 6, 2015

    I don't really know how to investigate that failure. Perhaps David wants to look into it?

    @vstinner
    Copy link
    Member

    vstinner commented Jan 6, 2015

    I don't really know how to investigate that failure.

    I don't think that it's specific to AIX, it just depends on the list of users on your system. On my Fedora 21, I have many users which have an empty home directory:

    • nobody
    • dbus
    • polkitd
    • usbmuxd
    • nm-openconnect
    • tcpdump
    • qemu
    • radvd
    • systemd-journal-gateway
    • systemd-timesync
    • systemd-network
    • systemd-resolve
    • systemd-bus-proxy

    If the test pick one of this user instead of a user with a non-empty home directory, the test fails with the same error. I propose a simple patch: test_pathlib_empty_homedir.patch

    @pitrou
    Copy link
    Member Author

    pitrou commented Jan 10, 2015

    Victor, your patch sounds ok to me.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 10, 2015

    New changeset 63dac5212552 by Victor Stinner in branch 'default':
    Issue bpo-19776: Fix test_pathlib.test_expanduser()
    https://hg.python.org/cpython/rev/63dac5212552

    @vstinner
    Copy link
    Member

    Ok, test_pathlib now pass on the buildbot "PPC64 AIX 3.x". I close the issue. Nice enhancement of the Path object.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants