Bug pyarrow implementation of str.fullmatch matches partial string. issue #56652

[JackCollins91]

• closes BUG: Pyarrow implementation of str.fullmatc matches partial string #56652
• Tests updated and passed
• All code checks passed

Changed array.py: Makes Series(["abc$abc]).str.fullmatch("abc\$") give the same result as when dtype = pyarrow.string as opposed to dtype = str.

Issue reporter (Issue-#56652) requested change "//$" to "\$", but this resulted in DepreciationWarnng in pytest, so used "\\$" instead.

Change test_arrow.py: updated test_str_fullmatch to account for edge cases where string starts with and ends with literal $ as well as ends with the string.

[asishm]

thanks for the PR!

Grepping the codebase for '//\$' gives

./pandas/core/arrays/arrow/array.py
./pandas/core/arrays/string_arrow.py

Testing further - it looks like the first affects dtype=pd.ArrowDtype(pa.string()) while the second one affects dtype="string[pyarrow]". The fix should be applied to both of the above files.

Also while the test method states test_str_fullmatch it seems to be testing ser.str.match instead. The fullmatch tests are present in ./pandas/tests/strings/test_find_replace.py

[JackCollins91]

Thanks @asishm , I can shortly make a commit which makes the changes in ./pandas/core/arrays/string_arrow.py
and also updates the fullmatch test in ./pandas/tests/strings/test_find_replace.py

To clarify, from testing, the changes to ./pandas/core/arrays/arrow/array.py also affect test_str_fullmatch even though it does so via str.fullmatch().

[JackCollins91]

Updated PR.
Question: For consistency, I formatted test_fullmatch_dollar_literal() to match other unit tests in .py - Should I instead aim to implement @pytest.mark.parametrize, which is inconsistent with its neighbour tests, but more in-line with developer guidelines?

[mroeschke]

Thanks could you add a whatsnew note in v2.2.0.rst under the Strings section?

[JackCollins91]

Thanks @mroeschke that update to v2.2.0.rst is added now.

[JackCollins91]

Resolved merge conflict by rebasing off upstream main.

[mroeschke]

Thanks @JackCollins91

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout 2.2.x
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 dc94b987a0c6920c0e14351372a374d566313cef

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #56691: Bug pyarrow implementation of str.fullmatch matches partial string. issue #56652'

4. Push to a named branch:

git push YOURFORK 2.2.x:auto-backport-of-pr-56691-on-2.2.x

5. Create a PR against branch 2.2.x, I would have named this PR:

"Backport PR #56691 on branch 2.2.x (Bug pyarrow implementation of str.fullmatch matches partial string. issue #56652)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.