Skip to content

bpo-38409: Grammatically correct help documentation of str.strip() #16681

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
Closed
wants to merge 3 commits into from
Closed

bpo-38409: Grammatically correct help documentation of str.strip() #16681

wants to merge 3 commits into from

Conversation

hansrajdas
Copy link
Contributor

@hansrajdas hansrajdas commented Oct 9, 2019 •
edited
Loading

  • remove should be removed as done for lstrip and rstrip help docs

strip help doc in python2 was correct but it was updated in python3:
python2.7

strip(...)
    S.strip([chars]) -> string or unicode

    Return a copy of the string S with leading and trailing
    whitespace removed.
    If chars is given and not None, remove characters in chars instead.
    If chars is unicode, S will be converted to unicode before stripping

python3.7

strip(self, chars=None, /)
    Return a copy of the string with leading and trailing whitespace remove.

    If chars is given and not None, remove characters in chars instead.

https://bugs.python.org/issue38409

@hansrajdas
bpo-38409: Grammatically correct help documentation of str.strip()
eff548c 
* `remove` should be `removed` as done for lstrip and rstrip help docs
@hansrajdas hansrajdas marked this pull request as ready for review October 9, 2019 19:50
ammaraskar
ammaraskar reviewed Oct 9, 2019
View reviewed changes
Misc/NEWS.d/next/Documentation/2019-10-10-01-00-29.bpo-38409.UQgQk1.rst Outdated
@@ -0,0 +1 @@
Grammatically corrected help(str.strip) documentation. Patch by Hansraj Das
Copy link
Member

Choose a reason for hiding this comment

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

Trivial fixes like these don't need a news blurb. I'm gonna apply to the skip-news label if you wanna take it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted news blurb file. Thank you!

ammaraskar
ammaraskar reviewed Oct 9, 2019
View reviewed changes
Objects/clinic/unicodeobject.c.h
@@ -582,7 +582,7 @@ PyDoc_STRVAR(unicode_strip__doc__,
"strip($self, chars=None, /)\n"
"--\n"
"\n"
"Return a copy of the string with leading and trailing whitespace remove.\n"
"Return a copy of the string with leading and trailing whitespace removed.\n"
Copy link
Member

Choose a reason for hiding this comment

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

This file is automatically generated using argument clinic, you need to regenerate it using make regen-all or make clinic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

executed make clinic

@zware
Copy link
Member

zware commented Oct 9, 2019

Considering that this is incorrect and was taken without attribution or approval of the original author, I'm closing this in favor of GH-16682. Since the original author hasn't signed the CLA, the usability of this patch is murky anyway.

Thanks for contributing, @hansrajdas, but when the reporter has offered a patch on bugs.python.org please give them a chance to either submit their own PR or refuse to do so; this one was submitted only yesterday.

@zware zware closed this Oct 9, 2019
@hansrajdas
Copy link
Contributor Author

Thanks for contributing, @hansrajdas, but when the reporter has offered a patch on bugs.python.org please give them a chance to either submit their own PR or refuse to do so; this one was submitted only yesterday.

Yes, I saw this bug reported yesterday itself(maked myself in that bug also yesterday) so waited one day if original author wants to send an PR. Sorry, I should have waited more!

@zware
Copy link
Member

zware commented Oct 9, 2019

The documented suggested wait time is one week of no response.

@hansrajdas
Copy link
Contributor Author

The documented suggested wait time is one week of no response.

Thank you @zware for guidance, I was not aware of this. Will always follow this guideline ever in future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ammaraskar ammaraskar ammaraskar approved these changes

Assignees
No one assigned
Labels
awaiting core review skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@hansrajdas @zware @ammaraskar @the-knights-who-say-ni @bedevere-bot