bpo-39939: Add str.removeprefix and str.removesuffix#18939
bpo-39939: Add str.removeprefix and str.removesuffix#18939vstinner merged 39 commits intopython:masterfrom
Conversation
serhiy-storchaka
left a comment
There was a problem hiding this comment.
The code LGTM. But I am not sure about adding these method for bytearray.
Needed also updates of the documentation.
Currently the bytearray api is almost a strict superset of the bytes api: >>> set(dir(bytes)) - set(dir(bytearray))
{'__getnewargs__'}and I didn't want to break that.
I will work on the docs tonight and tomorrow. |
|
Thank you. All this LGTM, and I am impressed by the quality of code from a new contributor. I do not press the "Approve" button only because I did not follow the discussion. You need an approve from a core developer more interested in this feature. Please add also a What's New entry. And add your name in Misc/ACKS. |
Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com>
sweeneyde
left a comment
There was a problem hiding this comment.
Change 'cut" --> "remove" in NEWS and whatsnew
Misc/NEWS.d/next/Core and Builtins/2020-03-11-19-17-36.bpo-39939.NwCnAM.rst
Outdated
Show resolved
Hide resolved
…39.NwCnAM.rst Change method names in NEWS
Misc/NEWS.d/next/Core and Builtins/2020-03-11-19-17-36.bpo-39939.NwCnAM.rst
Outdated
Show resolved
Hide resolved
|
Does someone know how to make the docs escape the slice |
vstinner
left a comment
There was a problem hiding this comment.
Overall, I like the change. Here is my second review on the doc, which is IMO the most important part here ;-)
Misc/NEWS.d/next/Core and Builtins/2020-03-11-19-17-36.bpo-39939.NwCnAM.rst
Outdated
Show resolved
Hide resolved
Sadly, your documentation syntax is correct. It's a bug in the "make suspicious" target of Doc/Makefile: We could try to fix "make suspicious". But first I suggest you to write |
|
Congrats @sweeneyde, I merged your PR! Maybe the documentation could still be enhanced even more, but I think that it's now good enough for an alpha release ;-) I built the documentation locally to check if it is rendered correctly and it was the case. I didn't see any obvious Sphinx syntax issue. |
|
| to easily remove an unneeded prefix or a suffix from a string. Corresponding | ||
| ``bytes``, ``bytearray``, and ``collections.UserString`` methods have also been | ||
| added. See :pep:`616` for a full description. (Contributed by Dennis Sweeney in | ||
| :issue:`18939`.) |
There was a problem hiding this comment.
Right, it's a typo error: @sweeneyde: can you please propose a PR to fix the typo?
There was a problem hiding this comment.
Or @elazarg: Do you want to propose a PR to fix the typo?
There was a problem hiding this comment.
Sure. I thought it might be overkill :)
There was a problem hiding this comment.
Oops -- It looks like that's the GitHub PR number rather than the bpo number. I can't make a PR tonight so feel free to change it. If not, I can fix it tomorrow.
https://bugs.python.org/issue39939