Skip to content

bpo-25433: Align bytearray strip methods to those found in byteobject.c#14771

Open
rompe wants to merge 3 commits into
python:mainfrom
rompe:bgo-25433-streamline-bytearray-strip-methods
Open

bpo-25433: Align bytearray strip methods to those found in byteobject.c#14771
rompe wants to merge 3 commits into
python:mainfrom
rompe:bgo-25433-streamline-bytearray-strip-methods

Conversation

@rompe
Copy link
Copy Markdown

@rompe rompe commented Jul 14, 2019 •
edited by bedevere-bot
Loading

According to bpo-25433, knowledge about whitespace should be implemented only once.

bytearray.strip, .rstrip, .lstrip all had their own definitions of whitespace.
The implementations for bytes is cleaner and a also a little bit faster, so I adopted the code from there and modified it to work with bytearrays and to fulfil the documentation which states that for bytearrays these functions always return copies, even if no changes have been applied.

I checked for performance regressions:

Current master:

./python -m timeit -s 'bla = bytearray(b"  foo  ")' 'bla.lstrip()'
1000000 loops, best of 5: 245 nsec per loop
./python -m timeit -s 'bla = bytearray(b"  foo  ")' 'bla.rstrip()'
1000000 loops, best of 5: 245 nsec per loop
./python -m timeit -s 'bla = bytearray(b"  foo  ")' 'bla.strip()'
1000000 loops, best of 5: 260 nsec per loop

Using this branch:

./python -m timeit -s 'bla = bytearray(b"  foo  ")' 'bla.lstrip()'
1000000 loops, best of 5: 235 nsec per loop
./python -m timeit -s 'bla = bytearray(b"  foo  ")' 'bla.rstrip()'
1000000 loops, best of 5: 235 nsec per loop
./python -m timeit -s 'bla = bytearray(b"  foo  ")' 'bla.strip()'
1000000 loops, best of 5: 239 nsec per loop

Timings using custom byte sequences to strip showed no differences at all between the two branches, no matter if one or multiple characters long.

https://bugs.python.org/issue25433

Ulf Rompe added 2 commits July 13, 2019 16:55
Align bytearray.strip, .rstrip, .lstrip to their bytes counterparts.
1c5834d 
The previous implementations differed a lot from the the ones found in
bytesobject.c and all three of them included hardcoded lists of
whitespace characters. Knowledge about whitespace already exists in
pyctype.c and should not be duplicated.
Update clinic signatures.
c016eb7
@the-knights-who-say-ni
Copy link
Copy Markdown

the-knights-who-say-ni commented Jul 14, 2019

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).

Our records indicate we have not received your 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.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

@rompe rompe mentioned this pull request Jul 14, 2019
Closed
@mangrisano
Copy link
Copy Markdown
Contributor

mangrisano commented Jul 14, 2019

/cc @tiran @serhiy-storchaka @pitrou

@eendebakpt eendebakpt mentioned this pull request Aug 31, 2022
Closed
@iritkatriel iritkatriel added the performance Performance or resource usage label Aug 31, 2022
@serhiy-storchaka
Copy link
Copy Markdown
Member

serhiy-storchaka commented Mar 2, 2024

I think that most of the code can be moved into bytes_methods.c and shared between bytes and bytearray implementations.

@bedevere-app bedevere-app Bot mentioned this pull request Apr 15, 2025
Open
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 7, 2026

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

awaiting review performance Performance or resource usage stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@rompe @the-knights-who-say-ni @mangrisano @serhiy-storchaka @iritkatriel @ezio-melotti @bedevere-bot