bpo-7940: add support for negative end positions to re.finditer and re.findall by anilbey · Pull Request #14744 · python/cpython

anilbey · 2019-07-13T13:38:12Z

add support for negative end positions to re.finditer and re.findall

https://bugs.python.org/issue7940

the-knights-who-say-ni · 2019-07-13T13:38:14Z

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!

isidentical

LGTM

ezio-melotti · 2019-07-13T14:39:53Z

+    if (start < 0)
+        start += length;
    if (start < 0)
        start = 0;


In re.compile('.').findall("abcd", -10, -3), the first if will set start to -6 which is not a valid index, and the second if is necessary because it will set it to 0. There are currently no tests for this scenario, so I would add a couple.

bedevere-bot · 2019-07-13T14:40:09Z

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

anilbey · 2019-07-14T12:14:12Z

I have made the requested changes; please review again.

bedevere-bot · 2019-07-14T12:14:15Z

Thanks for making the requested changes!

@ezio-melotti: please review the changes made to this pull request.

rhettinger · 2019-08-22T05:31:09Z

I've looked at this again and am still not inclined to accept it.

If someone else really believes this is a necessary API expansion and wants to move it forward, I won't object.

csabella · 2020-02-04T01:11:40Z

@ezio-melotti should this be merged or should there be further discussion?

anilbey · 2022-07-16T09:51:05Z

Just rebased at another EuroPython sprint after 3 years :)

anilbey · 2022-07-16T10:00:18Z

Quick summary

the current implementation accepts negative indices but silently truncates them to 0, which I doubt is the expected behaviour
this change allows negative indices to be used the same way they work in other collections
it enables consistency with the regex module
however it introduces a behavioural change

anilbey · 2022-07-18T08:48:41Z

This is a breaking change. A FutureWarning should be emitted for 2 releases prior to changing the behavior.

Hi @serhiy-storchaka is that something to be done in the code of this PR? How can we emit a FutureWarning?

iritkatriel

See #14744 (comment)

bedevere-bot · 2022-11-25T11:27:59Z

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

anilbey · 2023-03-15T10:28:14Z

@serhiy-storchaka @iritkatriel I have made the requested changes; please review again. :)

bedevere-bot · 2023-03-15T10:28:19Z

Thanks for making the requested changes!

@iritkatriel, @ezio-melotti, @isidentical: please review the changes made to this pull request.

anilbey · 2023-03-15T10:34:18Z

@isidentical since your last review I only added the FutureWarnings.

….findall

anilbey · 2023-08-16T13:57:46Z

Future Warning is emitted in #107105

dg-pb · 2024-06-27T18:28:11Z

Maybe it would be worth having global macro for this?
There is one in unicodeobject.c:ADJUST_INDICES, one in bytes_methods.c:ADJUST_INDICES and the same is done here. The same could be used when dealing with sequence indices too.

anilbey · 2024-06-27T22:00:25Z

Hi @dg-pb,

Thank you for your reply. This PR has been open for about 5 years now. Are you a core developer, and would you be interested in seeing this change merged?

dg-pb · 2024-06-27T23:41:28Z

Hi @dg-pb,

Thank you for your reply. This PR has been open for about 5 years now. Are you a core developer, and would you be interested in seeing this change merged?

I am not. Just decided to go through old PRs and decided to offer my thoughts for this one. Hopefully someone familiar with re can resolve whether to close or implement this.

github-actions · 2026-04-07T00:20:54Z

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

the-knights-who-say-ni added the CLA not signed label Jul 13, 2019

bedevere-bot added the awaiting review label Jul 13, 2019

isidentical approved these changes Jul 13, 2019

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Jul 13, 2019

ezio-melotti requested changes Jul 13, 2019

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting core review labels Jul 13, 2019

rhettinger self-assigned this Jul 13, 2019

bedevere-bot added awaiting change review and removed awaiting changes labels Jul 14, 2019

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Jul 15, 2019

rhettinger removed their assignment Aug 22, 2019

ezio-melotti approved these changes Sep 9, 2019

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Sep 9, 2019

AlexWaygood added the topic-regex label Apr 19, 2022

ezio-melotti removed the CLA signed label Jul 13, 2022

anilbey force-pushed the bpo7940 branch 2 times, most recently from a749312 to a7b0605 Compare July 16, 2022 09:50

kumaraditya303 assigned ezio-melotti Jul 16, 2022

iritkatriel requested changes Nov 25, 2022

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting merge labels Nov 25, 2022

anilbey force-pushed the bpo7940 branch 3 times, most recently from 73147f2 to b3dfb5a Compare March 15, 2023 10:00

bedevere-bot added awaiting change review and removed awaiting changes labels Mar 15, 2023

bedevere-bot requested review from ezio-melotti, iritkatriel and isidentical March 15, 2023 10:28

anilbey and others added 4 commits July 23, 2023 10:46

bpo7940: add support for negative end positions to re.finditer and re…

f47dad7

….findall

bpo7940: added test cases adressing the review

75e7c5f

emit FutureWarning for negative start/end indices

530afdf

test_bug_7940: assure warnings are effective via assertWarns

8bc3fb8

anilbey force-pushed the bpo7940 branch from b3dfb5a to 8bc3fb8 Compare July 23, 2023 08:47

tlynn mannequin mentioned this pull request Apr 19, 2022

re.finditer and re.findall should support negative end positions #52188

Open

Merge branch 'main' into bpo7940

46349e5

python deleted a comment Apr 7, 2025

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 7, 2026

Uh oh!

Conversation

anilbey commented Jul 13, 2019 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

isidentical left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezio-melotti Jul 13, 2019

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Jul 13, 2019

Uh oh!

anilbey commented Jul 14, 2019

Uh oh!

bedevere-bot commented Jul 14, 2019

Uh oh!

rhettinger commented Aug 22, 2019

Uh oh!

csabella commented Feb 4, 2020

Uh oh!

anilbey commented Jul 16, 2022

Uh oh!

anilbey commented Jul 16, 2022

Quick summary

Uh oh!

anilbey commented Jul 18, 2022

Uh oh!

iritkatriel left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Nov 25, 2022

Uh oh!

anilbey commented Mar 15, 2023

Uh oh!

bedevere-bot commented Mar 15, 2023

Uh oh!

anilbey commented Mar 15, 2023

Uh oh!

anilbey commented Aug 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dg-pb commented Jun 27, 2024

Uh oh!

anilbey commented Jun 27, 2024

Uh oh!

dg-pb commented Jun 27, 2024

Uh oh!

github-actions Bot commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

anilbey commented Jul 13, 2019 •

edited by bedevere-bot

Loading

isidentical left a comment •

edited

Loading

anilbey commented Aug 16, 2023 •

edited

Loading