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

Jump to bottom

gh-78707: deprecate passing >1 argument to PurePath.[is_]relative_to() #94469

Merged
merged 8 commits into from Dec 17, 2022
Merged

gh-78707: deprecate passing >1 argument to PurePath.[is_]relative_to() #94469

merged 8 commits into from Dec 17, 2022

Conversation

barneygale
Copy link
Contributor

@barneygale barneygale commented Jun 30, 2022 •
edited by miss-islington

This brings relative_to() and is_relative_to() more in line with other pathlib methods like rename() and symlink_to().

Resolves #78707.

Automerge-Triggered-By: GH:brettcannon

@barneygale
pythongh-78707: deprecate passing >1 argument to `PurePath.[is_]relat…
594bd8f 
…ive_to()`

This brings `relative_to()` and `is_relative_to()` more in line with other
pathlib methods like `rename()` and `symlink_to()`.
@barneygale barneygale requested a review from brettcannon as a code owner Jun 30, 2022
brettcannon
brettcannon requested changes Jul 4, 2022
View changes
Copy link
Member

@brettcannon brettcannon left a comment

While I support the idea, I think there are some ergonomic tweaks we can make the to new call signature. The docs also need deprecation warnings.

Lib/pathlib.py Outdated Show resolved Hide resolved
Doc/library/pathlib.rst Show resolved Hide resolved
@bedevere-bot
Copy link

bedevere-bot commented Jul 4, 2022

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@barneygale
Copy link
Contributor Author

barneygale commented Jul 9, 2022

I have made the requested changes; please review again

@bedevere-bot
Copy link

bedevere-bot commented Jul 9, 2022

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from brettcannon Jul 9, 2022
brettcannon
brettcannon requested changes Oct 22, 2022
View changes
Copy link
Member

@brettcannon brettcannon left a comment

One change to make to use a newer deprecation mechanism, but otherwise LGTM!

Lib/pathlib.py Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

bedevere-bot commented Oct 22, 2022

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@barneygale
Copy link
Contributor Author

barneygale commented Nov 3, 2022

Will fix conflicts when #99031 lands.

@barneygale barneygale marked this pull request as draft Nov 3, 2022
@bedevere-bot bedevere-bot mentioned this pull request Nov 25, 2022
Closed
@barneygale barneygale marked this pull request as ready for review Nov 25, 2022
@barneygale
Copy link
Contributor Author

barneygale commented Nov 25, 2022

I have made the requested changes; please review again

@bedevere-bot
Copy link

bedevere-bot commented Nov 25, 2022

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from brettcannon Nov 25, 2022
@barneygale
Copy link
Contributor Author

barneygale commented Dec 4, 2022

Hey @JelleZijlstra, as Brett is winding down his involvement in pathlib, do you think you could review this, please? I ask because you commented on #78707 some time ago :) Thanks!

hugovk
hugovk reviewed Dec 4, 2022
View changes
Copy link
Member

@hugovk hugovk left a comment

A couple of documentation suggestions, and shall also include it in "What’s New In Python 3.12"> "Pending Removal in Python 3.14"?

https://docs.python.org/3.12/whatsnew/3.12.html#pending-removal-in-python-3-14

These will help increase visibility for users.

Doc/library/pathlib.rst Outdated Show resolved Hide resolved
Doc/library/pathlib.rst Outdated Show resolved Hide resolved
@barneygale @hugovk
Apply suggestions from code review
0374a35 
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@netlify
Copy link

netlify bot commented Dec 6, 2022 •
edited

✅ Deploy Preview for python-cpython-preview ready!

Name Link
🔨 Latest commit 0374a35
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/638fc037932d91000ada8424
😎 Deploy Preview https://deploy-preview-94469--python-cpython-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@barneygale
Copy link
Contributor Author

barneygale commented Dec 6, 2022

Thank you for the feedback!

@brettcannon brettcannon added the 🤖 automerge PR will be merged once it's been approved and all CI passed label Dec 17, 2022
@miss-islington miss-islington merged commit 5a991da into python:main Dec 17, 2022
20 checks passed
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Dec 18, 2022
@shihai1991
Merge remote-tracking branch 'origin/main' into bpo_34697
2878571 
* origin/main: (1306 commits)
  Correct CVE-2020-10735 documentation (python#100306)
  pythongh-100272: Fix JSON serialization of OrderedDict (pythonGH-100273)
  pythongh-93649: Split tracemalloc tests from _testcapimodule.c (python#99551)
  Docs: Use `PY_VERSION_HEX` for version comparison (python#100179)
  pythongh-97909: Fix markup for `PyMethodDef` members (python#100089)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythonGH-98831: Add DECREF_INPUTS(), expanding to DECREF() each stack input (python#100205)
  pythongh-78707: deprecate passing >1 argument to `PurePath.[is_]relative_to()` (pythonGH-94469)
  pythongh-99540: Constant hash for _PyNone_Type to aid reproducibility (pythonGH-99541)
  pythongh-100039: enhance __signature__ to work with str and callables (pythonGH-100168)
  pythongh-99830: asyncio: Document returns of remove_{reader,writer} (python#100302)
  "Compound statement" docs: Fix with-statement step indexing (python#100286)
  pythonGH-90043: Handle NaNs in COMPARE_OP_FLOAT_JUMP (pythonGH-100278)
  Improve stats presentation for calls. (pythonGH-100274)
  Better stats for `LOAD_ATTR` and `STORE_ATTR` (pythonGH-100295)
  pythongh-81057: Move the Cached Parser Dummy Name to _PyRuntimeState (python#100277)
  Document that zipfile's pwd parameter is a `bytes` object (python#100209)
  pythongh-99767: mark `PyTypeObject.tp_watched` as internal use only in table (python#100271)
  Fix typo in introduction.rst (python#100266)
  ...
carljm added a commit to carljm/cpython that referenced this pull request Dec 19, 2022
@carljm
Merge branch 'main' into fastmock
a9af20a 
* main:
  pythongh-89727: Fix os.walk RecursionError on deep trees (python#99803)
  Docs: Don't upload CI artifacts (python#100330)
  pythongh-94912: Added marker for non-standard coroutine function detection (python#99247)
  Correct CVE-2020-10735 documentation (python#100306)
  pythongh-100272: Fix JSON serialization of OrderedDict (pythonGH-100273)
  pythongh-93649: Split tracemalloc tests from _testcapimodule.c (python#99551)
  Docs: Use `PY_VERSION_HEX` for version comparison (python#100179)
  pythongh-97909: Fix markup for `PyMethodDef` members (python#100089)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythongh-99240: Reset pointer to NULL when the pointed memory is freed in argument parsing (python#99890)
  pythonGH-98831: Add DECREF_INPUTS(), expanding to DECREF() each stack input (python#100205)
  pythongh-78707: deprecate passing >1 argument to `PurePath.[is_]relative_to()` (pythonGH-94469)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugovk hugovk

@brettcannon brettcannon

Assignees
No one assigned
Labels
🤖 automerge PR will be merged once it's been approved and all CI passed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Path.relative_to() taking multiple arguments could be better documented
5 participants
@barneygale @bedevere-bot @brettcannon @hugovk @miss-islington