BUG: fix bug in str.fullmatch for Arrow backend with optional groups

[ptth222]

Trying to address #61072 since I created the issue. I've never contributed to pandas, but I have tried to follow what is in the guide.

Closes #61072

[mroeschke]

Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.

[jorisvandenbossche]

@ptth222 sorry you didn't get any feedback on the PR initially, but thanks for opening the issue and this PR! It looks like a good start to me, but will need to address some failing tests (and also add a new test case for what it is fixing)

[ptth222]

I added a test that tests the arrow fullmatch against the python fullmatch. I also might have found a mistake in the fullmatch test. "test_str_fullmatch" in tests/extension/test_arrow.py was calling the match method and not the fullmatch method, so I changed it to use fullmatch. I also had to change some of the expected values to match what should be expected from fullmatch.

I can't for the life of me figure out what the problem is with the new test I added, test_str_fullmatch_against_python_fullmatch. It is producing AssertionError: Series NA mask are different, but both series should be the exact same values and types. I have tried using both ArrowDtype(pa.string()) and result.dtype in the astype method, but still get this same error. I can't reproduce this on my own machine. If someone has any insights into what the issue could be, I would love to hear it.

I didn't add any more tests except for the one for fullmatch, but it might be a good idea to add the same kind of test for match, search, etc. Assuming we can get the issues straightened out.

[jorisvandenbossche]

I also might have found a mistake in the fullmatch test. "test_str_fullmatch" in tests/extension/test_arrow.py was calling the match method and not the fullmatch method, so I changed it to use fullmatch. I also had to change some of the expected values to match what should be expected from fullmatch.

Good catch! Thanks for updating that

I can't for the life of me figure out what the problem is with the new test I added, test_str_fullmatch_against_python_fullmatch. It is producing AssertionError: Series NA mask are different, but both series should be the exact same values and types.

The reason for this is because the ArrowDtype handles NA in the input Series differently, i.e. it propagates as missing to the result while the str dtype will propagate it as False. Initially I pushed a fix for that by removing the missing value from the test case. But eventually I moved the entire test to test_find_replace.py: there, it will automatically be run for the various string dtypes, including the ones backed by Python strings, and thus the expected result gets verified against Python that way.

[jorisvandenbossche]

I didn't add any more tests except for the one for fullmatch

I think we still need to add a test for the actual issue you reported. Because right now, if I undo the actual code fix in _str_fullmatch, then all updated tests still pass, indicating the current tests don't cover the bug you are trying to fix.

[jorisvandenbossche]

@ptth222 I pushed a small update to the parametrized test to include one case with the optional groups (or I don't know what the exact regex terminology is for this) from the issue, and add a whatsnew note.
Apologies for quickly pushing myself, but I want to include this fix in the upcoming 2.3.3. release that I am preparing right now.

[ptth222]

Sounds good to me. Thank you for the assistance.

[jorisvandenbossche]

Thanks @ptth222!