Repository navigation
[Python] Return every group from Regex.all_matches - #40563
Conversation
Regex.all_matches built its output from range(m.lastindex + 1), but lastindex is the index of the last group that matched (or the outermost group that closed last), not the number of groups. A regex without groups failed with a TypeError, and nested groups or an unmatched trailing optional group were dropped from the output. Use the entire match followed by m.groups(), which has one entry per group, with None for a group that did not participate, as Java's AllMatches does with null. Fixes apache#40562
|
Assigning reviewers: R: @tvalentyn for label python. This pull request likely touches a core component ("core" label). Please review with scrutiny. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
I can merge once test suites finish, please ping if i don't get back to it in a day. |
|
(don't worry if there are unrelated failures in the CI) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40563 +/- ##
=============================================
+ Coverage 56.16% 79.93% +23.76%
=============================================
Files 1124 488 -636
Lines 178336 82347 -95989
Branches 1489 0 -1489
=============================================
- Hits 100170 65826 -34344
+ Misses 75645 16521 -59124
+ Partials 2521 0 -2521
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Fixes #40562
Regex.all_matchesbuilt each output fromrange(m.lastindex + 1).lastindexis the index of the last group that matched, and with nested groups it is the outer group that closed last. It isNonewhen the regex has no groups. Because of this:Each output is now
[m.group(), *m.groups()]: the entire match, then one entry per group of the regex, withNonefor a group that did not take part in the match. This is what Java'sRegex.AllMatchesreturns (it addsnullfor such groups), and it is the same change #40399 made forRegex.find_all(..., Regex.ALL). Output for a regex whose groups all match, like the existing tests and theregex_all_matchessnippet, does not change.With this change the pipelines above print
['ab', 'ab', 'a', 'b'],['a', 'a', None]and['aaa'].I added
test_all_matched_returns_every_grouptoutil_test.py. It runs the three cases in aTestPipeline. It fails with theTypeErrorbefore the change and passes after it. I also updated the docstring to describe the output.Testing:
pytest apache_beam/transforms/util_test.py(157 passed, 3 skipped)pytest apache_beam/examples/snippets/transforms/elementwise/ -k regexyapf --diffandruff checkon the changed filesThank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123orfixes #456).CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.