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

bpo-323042: Document how to remove a suffix of a pathlib.Path #8655

Merged

Conversation

sotte
Copy link
Contributor

@sotte sotte commented Aug 3, 2018

bpo-323042: Document how to remove a suffix of a pathlib.Path

I didn't realize that you can remove a suffix with the with_suffix function of the Path class of pathlib and I always used a little utility function that I wrote.

I wanted to add that functionality (removing of a suffix) and submit a PR but then I saw that with_suffix has you covered already. I'm just documenting this feature with this PR.

https://bugs.python.org/issue323042

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

When your account is ready, please add a comment in this pull request
and a Python core developer will remove the CLA not signed label
to make the bot check again.

You can check yourself
to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

Copy link
Member

@berkerpeksag berkerpeksag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also remove the outdated comment in Lib/pathlib.py?

# XXX if suffix is None, should the current suffix be removed?

@sotte
Copy link
Contributor Author

sotte commented Aug 3, 2018

Sure. I also documented the empty string case in the docstring.

Lib/pathlib.py Outdated
"""Return a new path with the file suffix changed (or added, if none)."""
# XXX if suffix is None, should the current suffix be removed?
"""Return a new path with the file suffix changed. If the path
has no suffix, add `suffix`. If the given `suffix` is an empty
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: We don't use reST markup in docstrings.

@sotte sotte force-pushed the improve_pathlib_with_suffix_documentation branch from 6b7a3e7 to 7a8d530 Compare August 3, 2018 19:12
@sotte
Copy link
Contributor Author

sotte commented Aug 3, 2018

Thanks for the feedback!

FYI: In some places in pathlib.py rst-style comments are used. One could clean that up at some point.

@sotte
Copy link
Contributor Author

sotte commented Aug 3, 2018

Should I squash the commits or is any other action required from my side?

@berkerpeksag berkerpeksag merged commit 46dc4e3 into python:master Aug 3, 2018
@miss-islington
Copy link
Contributor

Thanks @sotte for the PR, and @berkerpeksag for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-8661 is a backport of this pull request to the 3.7 branch.

@bedevere-bot
Copy link

GH-8662 is a backport of this pull request to the 3.6 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 3, 2018
(cherry picked from commit 46dc4e3)

Co-authored-by: Stefan Otte <stefan.otte@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 3, 2018
(cherry picked from commit 46dc4e3)

Co-authored-by: Stefan Otte <stefan.otte@gmail.com>
@berkerpeksag
Copy link
Member

Thanks!

Should I squash the commits or is any other action required from my side?

No, I was just waiting for tests to be completed.

@sotte
Copy link
Contributor Author

sotte commented Aug 3, 2018

Thanks! Was a lot of fun :)

berkerpeksag pushed a commit that referenced this pull request Aug 3, 2018
(cherry picked from commit 46dc4e3)

Co-authored-by: Stefan Otte <stefan.otte@gmail.com>
berkerpeksag pushed a commit that referenced this pull request Aug 3, 2018
(cherry picked from commit 46dc4e3)

Co-authored-by: Stefan Otte <stefan.otte@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants