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-39939: Add str.removeprefix and str.removesuffix #18939

Merged
merged 39 commits into from Apr 22, 2020

Conversation

@sweeneyde
Copy link
Contributor

@sweeneyde sweeneyde commented Mar 11, 2020

blurb-it bot and others added 3 commits Mar 11, 2020
@sweeneyde sweeneyde requested a review from rhettinger as a code owner Mar 11, 2020
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

The code LGTM. But I am not sure about adding these method for bytearray.

Needed also updates of the documentation.

Objects/bytesobject.c Show resolved Hide resolved
@sweeneyde
Copy link
Contributor Author

@sweeneyde sweeneyde commented Mar 12, 2020

But I am not sure about adding these method for bytearray.

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.

Needed also updates of the documentation.

I will work on the docs tonight and tomorrow.

sweeneyde added 7 commits Mar 12, 2020
@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Mar 12, 2020

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.

Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved
Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com>
Copy link
Contributor Author

@sweeneyde sweeneyde left a comment

Change 'cut" --> "remove" in NEWS and whatsnew

Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved
sweeneyde added 2 commits Mar 25, 2020
….NwCnAM.rst


Change method names in NEWS
@sweeneyde sweeneyde changed the title bpo-39939: Add str.cutprefix and str.cutsuffix bpo-39939: Add str.removeprefix and str.removesuffix Mar 25, 2020
sweeneyde added 3 commits Apr 9, 2020
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.9.rst Show resolved Hide resolved
Doc/library/stdtypes.rst Show resolved Hide resolved
sweeneyde added 2 commits Apr 22, 2020
@sweeneyde
Copy link
Contributor Author

@sweeneyde sweeneyde commented Apr 22, 2020

Does someone know how to make the docs escape the slice string[:len(string)-len(suffix)]? I'm having trouble with Sphinx thinking that :len is a special kind of role.

Copy link
Member

@vstinner vstinner left a comment

Overall, I like the change. Here is my second review on the doc, which is IMO the most important part here ;-)

Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/library/stdtypes.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved
Objects/bytearrayobject.c Outdated Show resolved Hide resolved
Objects/bytearrayobject.c Outdated Show resolved Hide resolved
Objects/bytearrayobject.c Outdated Show resolved Hide resolved
@vstinner
Copy link
Member

@vstinner vstinner commented Apr 22, 2020

Does someone know how to make the docs escape the slice string[:len(string)-len(suffix)]? I'm having trouble with Sphinx thinking that :len is a special kind of role.

Sadly, your documentation syntax is correct. It's a bug in the "make suspicious" target of Doc/Makefile:

Makefile:115: recipe for target 'suspicious' failed

We could try to fix "make suspicious". But first I suggest you to write string[:-len(suffix)] instead: it's the syntax used in the PEP 616 and it's correct. I'm not sure why you chose to explicit the string length. I expect that "make suspicious" will not complain on :- which doesn't look like a Sphinx markup.

sweeneyde added 3 commits Apr 22, 2020
@vstinner vstinner merged commit a81849b into python:master Apr 22, 2020
9 checks passed
9 checks passed
Docs
Details
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20200422.40 succeeded
Details
bedevere/issue-number Issue number 39939 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vstinner
Copy link
Member

@vstinner vstinner commented Apr 22, 2020

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.

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Apr 22, 2020

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Fedora Stable LTO + PGO 3.x has failed when building commit a81849b.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/64/builds/656) and take a look at the build logs.
  4. Check if the failure is related to this commit (a81849b) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/64/builds/656

Failed tests:

  • test_concurrent_futures

Failed subtests:

  • test_killed_child - test.test_concurrent_futures.ProcessPoolSpawnProcessPoolExecutorTest

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

406 tests OK.

10 slowest tests:

  • test_concurrent_futures: 2 min 39 sec
  • test_multiprocessing_spawn: 1 min 43 sec
  • test_mailbox: 1 min 22 sec
  • test_nntplib: 1 min 21 sec
  • test_multiprocessing_forkserver: 1 min 15 sec
  • test_shelve: 1 min 3 sec
  • test_multiprocessing_fork: 1 min 3 sec
  • test_largefile: 51.6 sec
  • test_asyncio: 48.8 sec
  • test_signal: 47.1 sec

1 test failed:
test_concurrent_futures

14 tests skipped:
test_devpoll test_gdb test_ioctl test_kqueue test_msilib
test_ossaudiodev test_startfile test_tix test_tk test_ttk_guionly
test_winconsoleio test_winreg test_winsound test_zipfile64

1 re-run test:
test_concurrent_futures

Total duration: 5 min 46 sec

Click to see traceback logs
Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.lto-pgo/build/Lib/test/test_concurrent_futures.py", line 130, in tearDown
    self.executor.shutdown(wait=True)
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.lto-pgo/build/Lib/concurrent/futures/process.py", line 724, in shutdown
    self._executor_manager_thread_wakeup.wakeup()
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.lto-pgo/build/Lib/concurrent/futures/process.py", line 80, in wakeup
    self._writer.send_bytes(b"")
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.lto-pgo/build/Lib/multiprocessing/connection.py", line 205, in send_bytes
    self._send_bytes(m[offset:offset + size])
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.lto-pgo/build/Lib/multiprocessing/connection.py", line 416, in _send_bytes
    self._send(header + buf)
  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-x86_64.lto-pgo/build/Lib/multiprocessing/connection.py", line 373, in _send
    n = write(self._handle, buf)
OSError: [Errno 9] Bad file descriptor
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`.)

This comment has been minimized.

@elazarg

elazarg May 28, 2020
Contributor

This is not 18939, which is about "Venv docs regarding original python install". Shouldn't this be 39939?

This comment has been minimized.

@vstinner

vstinner May 28, 2020
Member

Right, it's a typo error: @sweeneyde: can you please propose a PR to fix the typo?

This comment has been minimized.

@vstinner

vstinner May 28, 2020
Member

Or @elazarg: Do you want to propose a PR to fix the typo?

This comment has been minimized.

@elazarg

elazarg May 28, 2020
Contributor

Sure. I thought it might be overkill :)

This comment has been minimized.

@sweeneyde

sweeneyde May 28, 2020
Author Contributor

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.

This comment has been minimized.

@elazarg

elazarg May 28, 2020
Contributor

Done.

vegerot added a commit to vegerot/cpython that referenced this pull request Jun 10, 2020
Added str.removeprefix and str.removesuffix methods and corresponding
bytes, bytearray, and collections.UserString methods to remove affixes
from a string if present. See PEP 616 for a full description.
vegerot added a commit to vegerot/cpython that referenced this pull request Jun 10, 2020
Added str.removeprefix and str.removesuffix methods and corresponding
bytes, bytearray, and collections.UserString methods to remove affixes
from a string if present. See PEP 616 for a full description.
gmelikov added a commit to gmelikov/cpython that referenced this pull request Aug 22, 2020
Added str.removeprefix and str.removesuffix methods and corresponding
bytes, bytearray, and collections.UserString methods to remove affixes
from a string if present. See PEP 616 for a full description.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.