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.

Sign up for GitHub

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

Jump to bottom

Add pathlib.Path.hardlink_to() #84131

Closed
barneygale mannequin opened this issue Mar 13, 2020 · 9 comments · Fixed by #92505
Closed

Add pathlib.Path.hardlink_to() #84131

barneygale mannequin opened this issue Mar 13, 2020 · 9 comments · Fixed by #92505
Labels
3.10 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@barneygale
Copy link
Mannequin

barneygale mannequin commented Mar 13, 2020

BPO 39950
Nosy @gpshead, @miss-islington, @tirkarthi, @barneygale, @escape0707
PRs
  • bpo-39950: add pathlib.Path.hardlink_to() method that supersedes link_to() #18909
  • bpo-39950: Fix deprecation warning in test for pathlib.Path.link_to() #26155
  • [3.10] bpo-39950: Fix deprecation warning in test for pathlib.Path.link_to() (GH-26155) #26158
  • [3.10] bpo-39950: Fix deprecation warning in test for pathlib.Path.link_to() (GH-26155) #26178
    		•

    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 2021-06-13.15:49:54.023>
created_at = <Date 2020-03-13.00:50:05.346>
labels = ['type-feature', 'library', '3.10']
title = 'Add pathlib.Path.hardlink_to()'
updated_at = <Date 2021-06-13.15:49:54.022>
user = 'https://github.com/barneygale'

    bugs.python.org fields:

    activity = <Date 2021-06-13.15:49:54.022>
actor = 'barneygale'
assignee = 'none'
closed = True
closed_date = <Date 2021-06-13.15:49:54.023>
closer = 'barneygale'
components = ['Library (Lib)']
creation = <Date 2020-03-13.00:50:05.346>
creator = 'barneygale'
dependencies = []
files = []
hgrepos = []
issue_num = 39950
keywords = ['patch']
message_count = 9.0
messages = ['364060', '365275', '385199', '385458', '385471', '385475', '391734', '392779', '393765']
nosy_count = 5.0
nosy_names = ['gregory.p.smith', 'miss-islington', 'xtreak', 'barneygale', 'escape0707']
pr_nums = ['18909', '26155', '26158', '26178']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue39950'
versions = ['Python 3.10']
    @barneygale
    Copy link
    Mannequin Author

    barneygale mannequin commented Mar 13, 2020

    Per bpo-39291, the argument order for pathlib.Path.link_to() is inconsistent with symlink_to() and its own documentation.

    This ticket covers adding a new hardlink_to() method with the correct argument order, and deprecating link_to().

    Discussion on python-dev: https://mail.python.org/archives/list/[email protected]/thread/7QPLYW36ZK6QTW4SV4FI6C343KYWCPAT/

    @barneygale barneygale mannequin added 3.9 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Mar 13, 2020
    @barneygale
    Copy link
    Mannequin Author

    barneygale mannequin commented Mar 29, 2020

    A question:

    For my patch, I need to include a Python version where Path.link_to() will become unavailable. I'm not entirely sure how this should be determined. Some factors in play:

    Can anyone suggest in which version of Python this function should be removed? Thanks.

    @escape0707
    Copy link
    Mannequin

    escape0707 mannequin commented Jan 18, 2021

    Maybe we could have the correct Path.hardlink implemented before removing or even deprecating the confusing Path.link_to? It will only help even if we don't remove the latter in a hurry.

    @barneygale
    Copy link
    Mannequin Author

    barneygale mannequin commented Jan 21, 2021

    Makes sense to me. Should I leave the documentation for link_to completely alone? With the addition of a similar function, I wonder if that may in itself cause confusion.

    @escape0707
    Copy link
    Mannequin

    escape0707 mannequin commented Jan 22, 2021

    For me, and as you've pointed out, the current doc of Path.link_to is already wrong and misleading. Perhaps a fix of the doc should be made as a first step.

    The doc uses the expression "Create a hard link pointing to a path named target."
    But comparing this to the doc of Path.symlink_to, which says "Make this path a symbolic link to target.",
    (and the doc of os.link, which says, "Create a hard link pointing to src named dst.",)
    the current doc should actually be "Create a hard link at target pointing to this path." Sadly the argument name can't be changed here already.

    And the doc of Path.symlink_to even have a helpful note: "Note: The order of arguments (link, target) is the reverse of os.symlink()’s."
    We could also add one for Path.link_to, too, like "Note: The order of arguments (link, target) is the consistent with os.link()’s.

    Based on that foundation, we could finally continue to implement the correct "Path.hardlink_to".

    We can refer to how people retained subprocess.call while adding a newer , preferred subprocess.run, by noting people to prefer Path.hardlink_to as it's more consistent with the pathlib and OOP, along with telling them the obscurity of Path.link_to and why it's left here for backward capability.

    @barneygale
    Copy link
    Mannequin Author

    barneygale mannequin commented Jan 22, 2021

    I've logged bpo-42999 to cover fixing the existing link_to() docs issues. PR incoming...

    @miss-islington
    Copy link
    Contributor

    New changeset f24e2e5 by Barney Gale in branch 'master':
    bpo-39950: add pathlib.Path.hardlink_to() method that supersedes link_to() (GH-18909)
    f24e2e5

    @tirkarthi
    Copy link
    Member

    The changes have introduced deprecation warnings in tests. It seems

    cpython/Lib/test/test_pathlib.py

    Line 1937 in ad106c6

    q.link_to(r)
    is not covered.

    ./python -Wall -m test test_pathlib
    0:00:00 load avg: 0.81 Run tests sequentially
    0:00:00 load avg: 0.81 [1/1] test_pathlib
    /root/cpython/Lib/pathlib.py:1265: DeprecationWarning: pathlib.Path.link_to() is deprecated and is scheduled for removal in Python 3.12. Use pathlib.Path.hardlink_to() instead.
    warnings.warn("pathlib.Path.link_to() is deprecated and is scheduled "
    /root/cpython/Lib/pathlib.py:1265: DeprecationWarning: pathlib.Path.link_to() is deprecated and is scheduled for removal in Python 3.12. Use pathlib.Path.hardlink_to() instead.
    warnings.warn("pathlib.Path.link_to() is deprecated and is scheduled "

    == Tests result: SUCCESS ==

    1 test OK.

    Total duration: 2.9 sec
    Tests result: SUCCESS

    Also it will be nice to point to the line causing deprecation instead of the library soruce code that will help users identify the deprecated usage. Using stacklevel=2 might help here https://docs.python.org/3/library/warnings.html#warnings.warn

    ./python -Wall -m test test_pathlib
    0:00:00 load avg: 1.82 Run tests sequentially
    0:00:00 load avg: 1.82 [1/1] test_pathlib
    /root/cpython/Lib/test/test_pathlib.py:1937: DeprecationWarning: pathlib.Path.link_to() is deprecated and is scheduled for removal in Python 3.12. Use pathlib.Path.hardlink_to() instead.
    q.link_to(r)
    /root/cpython/Lib/test/test_pathlib.py:1937: DeprecationWarning: pathlib.Path.link_to() is deprecated and is scheduled for removal in Python 3.12. Use pathlib.Path.hardlink_to() instead.
    q.link_to(r)

    == Tests result: SUCCESS ==

    1 test OK.

    Total duration: 4.3 sec
    Tests result: SUCCESS

    @barneygale barneygale mannequin closed this as completed May 15, 2021
    @barneygale barneygale mannequin reopened this May 15, 2021
    @gpshead
    Copy link
    Member

    gpshead commented May 16, 2021

    New changeset b913f47 by Miss Islington (bot) in branch '3.10':
    bpo-39950: Fix deprecation warning in test for pathlib.Path.link_to() (GH-26155) (GH-26178)
    b913f47

    @barneygale barneygale mannequin added 3.10 only security fixes and removed 3.9 only security fixes labels Jun 13, 2021
    @barneygale barneygale mannequin closed this as completed Jun 13, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    gpshead added a commit to gpshead/cpython that referenced this issue May 8, 2022
    @gpshead
    pythongh-84131: Remove the deprecated pathlib.Path.link_to method.
    04afd5e
    @gpshead gpshead linked a pull request May 8, 2022 that will close this issue
    gh-84131: Remove the deprecated pathlib.Path.link_to method. #92505
    Merged
    gpshead added a commit that referenced this issue May 10, 2022
    @gpshead @barneygale
    gh-84131: Remove the deprecated pathlib.Path.link_to method. (#92505)
    07b3492 
    Co-authored-by: Barney Gale <[email protected]>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.10 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging a pull request may close this issue.

    gh-84131: Remove the deprecated pathlib.Path.link_to method. gpshead/cpython
    3 participants
    @gpshead @tirkarthi @miss-islington