bpo-7940: add support for negative end positions to re.finditer and re.findall #14744

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

Modules/_sre.c

@@ -427,11 +427,15 @@ state_init(SRE_STATE* state, PatternObject* pattern, PyObject* string,
    }

    /* adjust boundaries */
+    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

isidentical

Re-approving the overall PR, but we need a regex expert on this @ezio-melotti @serhiy-storchaka

serhiy-storchaka · 2022-07-18T06:08:37Z

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

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.

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

isidentical approved these changes Jul 16, 2022

View reviewed changes

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-7940: add support for negative end positions to re.finditer and re.findall #14744

bpo-7940: add support for negative end positions to re.finditer and re.findall #14744

anilbey commented Jul 13, 2019 •

edited by bedevere-bot

Loading

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

isidentical left a comment •

edited

Loading

ezio-melotti Jul 13, 2019

bedevere-bot commented Jul 13, 2019

anilbey commented Jul 14, 2019

bedevere-bot commented Jul 14, 2019

rhettinger commented Aug 22, 2019

csabella commented Feb 4, 2020

anilbey commented Jul 16, 2022

anilbey commented Jul 16, 2022

isidentical left a comment

serhiy-storchaka commented Jul 18, 2022

anilbey commented Jul 18, 2022

iritkatriel left a comment

bedevere-bot commented Nov 25, 2022

anilbey commented Mar 15, 2023

bedevere-bot commented Mar 15, 2023

anilbey commented Mar 15, 2023

anilbey commented Aug 16, 2023 •

edited

Loading

dg-pb commented Jun 27, 2024

anilbey commented Jun 27, 2024

dg-pb commented Jun 27, 2024

bpo-7940: add support for negative end positions to re.finditer and re.findall #14744

Are you sure you want to change the base?

bpo-7940: add support for negative end positions to re.finditer and re.findall #14744

Conversation

anilbey commented Jul 13, 2019 • edited by bedevere-bot Loading

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

isidentical left a comment • edited Loading

Choose a reason for hiding this comment

ezio-melotti Jul 13, 2019

Choose a reason for hiding this comment

bedevere-bot commented Jul 13, 2019

anilbey commented Jul 14, 2019

bedevere-bot commented Jul 14, 2019

rhettinger commented Aug 22, 2019

csabella commented Feb 4, 2020

anilbey commented Jul 16, 2022

anilbey commented Jul 16, 2022

Quick summary

isidentical left a comment

Choose a reason for hiding this comment

serhiy-storchaka commented Jul 18, 2022

anilbey commented Jul 18, 2022

iritkatriel left a comment

Choose a reason for hiding this comment

bedevere-bot commented Nov 25, 2022

anilbey commented Mar 15, 2023

bedevere-bot commented Mar 15, 2023

anilbey commented Mar 15, 2023

anilbey commented Aug 16, 2023 • edited Loading

dg-pb commented Jun 27, 2024

anilbey commented Jun 27, 2024

dg-pb commented Jun 27, 2024

anilbey commented Jul 13, 2019 •

edited by bedevere-bot

Loading

isidentical left a comment •

edited

Loading

anilbey commented Aug 16, 2023 •

edited

Loading