Skip to content

Conversation

@chason
Copy link
Contributor

@chason chason commented Jan 25, 2018

The classes that inherit from PurePath do not have docstrings,
and therefore inherit the docstrings from PurePath. This is confusing
because a developer may assume that Path works similarly to PurePath,
which of course it doesn't. This adds a docstring to Path as well as
to the other PurePath children that is more indicative of their role.

https://bugs.python.org/issue31972

The classes that inherit from PurePath do not have docstrings,
and therefore inherit the docstrings from PurePath. This is confusing
because a developer may assume that Path works similarly to PurePath,
which of course it doesn't. This adds a docstring to Path as well as
to the other PurePath children that is more indicative of their role.
@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.

Thanks again to your contribution and we look forward to looking at it!

@chason
Copy link
Contributor Author

chason commented Jan 25, 2018

Not sure why this says my CLA is not signed, I have signed it.

class PurePosixPath(PurePath):
"""On a POSIX system, instantiating a PurePath should return this object.
However, you can also instantiate it directly on any system.
"""
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the patch!

In the stdlib we try to follow the guidelines laid out in PEP 257. Mainly, the first line of a docstring should be a very short description that stands alone, and more information follows after a blank line (this is very similar to the recommendations for git commit messages).

Taking inspiration from the rst docs, here the first line could be something like PurePath subclass for non-Windows systems., followed by a blank line then your original text.

Copy link
Member

@merwok merwok left a comment

Choose a reason for hiding this comment

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

Style suggestions in comments

@bedevere-bot
Copy link

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.

@merwok
Copy link
Member

merwok commented Jan 25, 2018

Did you put your github username in your bugs.python.org profile? I think that’s what the bot needs to check your CLA status.

@chason
Copy link
Contributor Author

chason commented Jan 26, 2018

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

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

@chason
Copy link
Contributor Author

chason commented Jan 26, 2018

Yes, I added my github username but it was just yesterday that I signed the CLA so I think it just hasn't been updated yet.

As for the changes you requested, I've made them. I also updated the existing docstring for class PurePath as it seemed to be within the purview of this bug as well. Let me know if I should leave it the way it was.

@merwok merwok merged commit dfa015c into python:master Feb 18, 2018
@miss-islington
Copy link
Contributor

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

@bedevere-bot
Copy link

@merwok: Please replace # with GH- in the commit message next time. Thanks!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 18, 2018
(cherry picked from commit dfa015c)

Co-authored-by: chason <chason@gmail.com>
@bedevere-bot
Copy link

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

@miss-islington
Copy link
Contributor

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 18, 2018
(cherry picked from commit dfa015c)

Co-authored-by: chason <chason@gmail.com>
@bedevere-bot
Copy link

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

miss-islington added a commit that referenced this pull request Feb 19, 2018
(cherry picked from commit dfa015c)

Co-authored-by: chason <chason@gmail.com>
miss-islington added a commit that referenced this pull request Feb 19, 2018
(cherry picked from commit dfa015c)

Co-authored-by: chason <chason@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants