Typo: String strip method docstring 'remove' -> 'removed'. #15464
Conversation
Seems it should be what the docs say: https://docs.python.org/3/library/stdtypes.html?highlight=strip#str.strip
|
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 we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for your contribution, we look forward to reviewing it! |
There was a problem hiding this comment.
Thanks for the PR @mbanerjeepalmer and welcome! Good catch.
Since this is correcting a docstring, I'll categorize it as being a documentation change. The typo correction looks good to me, but because this file is using Argument Clinic, we'll have to run Tools/clinic/clinic.py on it to update the checksum. That's why Travis is failing.
Regarding the CLA, if the status is not updated within 24 hours of signing it, you may have to refresh the status on GitHub manually: https://check-python-cla.herokuapp.com.
We'll have to wait for a core developer to review the changes before merging it (and for the CLA to be signed), but let me know if you have any questions in the meantime. If you haven't already, I'd recommend checking out the "Lifecycle of a Pull Request" section of the devguide.
|
Thanks for explaining @aeros167. I signed the CLA 24 hours ago but the checker hasn't updated yet. I'll look again tomorrow to ensure I've done everything correctly. |
Did you create an account on bugs.python.org as well? Once the "Contributor Form Received" status is updated on there, it should update on GitHub soon after. |
|
Looks like the CLA issue is resolved now. Regarding Argument Clinic, I'm afraid I'm not entirely clear what I need to do next. Could you point me to where I can find instructions? It's not clear which part of the docs relate to what I'm trying to do here. |
The main documentation for Argument Clinic is on https://docs.python.org/3/howto/clinic.html. I'm not particularly experienced with using Argument Clinic (as my PRs have not involved changes to C files). As far as I'm aware, all you should have to do is run clinic.py on the C header file to update the checksum using This should be done locally within the branch being used for the PR (in your case, "master", which is the default branch), commit the changes, and then push the changes directly to the PR branch. If you're not overly familiar with using Git, I can try to walk you through that process. Note: It's generally not advisable to open a PR from your master, as it makes it difficult to have multiple open PRs in the same repository. Before opening a new PR, I usually will update master, create a new branch, make the changes within it, commit, and then push the local repository to the remote origin. It should be okay for this PR, but I just wanted to let you know in case you weren't aware. (: Git Bootcamp: https://devguide.python.org/gitbootcamp/. The guide was mainly written in the context of contributing to CPython, but it's an excellent resource for Git usage in general. |
| @@ -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" | |||
There was a problem hiding this comment.
This is a generated file. Can you instead make the correction in Objects/unicodeobject.c and then run make clinic?
There was a problem hiding this comment.
Can you instead make the correction in Objects/unicodeobject.c and then run make clinic?
Ah, I wasn't aware that Argument Clinic could not be ran against the header file directly, but that would make sense. I've personally never had to use it before, so I've only briefly read over the documentation to try to help others.
Also, I was not aware of the existence of make clinic, I thought the only option was to explicitly use ./Tools/clinic/clinic.py <path>. It's not mentioned within the devguide as far as I can tell: https://devguide.python.org/search/?q=make+clinic&check_keywords=yes&area=default.
Should we perhaps mention it somewhere? Using make clinic is far more straightforward than using clinic.py on each of the C files that need it.
There was a problem hiding this comment.
Hmm, it looks like we also don't mention make regen-all anywhere in the devguide, which is probably the better target to remember since it includes make clinic. On the other hand neither is available on Windows, so we should probably also mention the clinic script directly.
|
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 |
|
@mbanerjeepalmer, please address the last comment by @zware. Thank you! |
|
This is a minor change, but seems to have been abandoned. I'm going to close this pull request. It can be reopened if work will continue on it or a new pull request can be opened to replace it. Thank you! |
Seems it should be what the docs say: https://docs.python.org/3/library/stdtypes.html?highlight=strip#str.strip