Skip to content

BUG: raise a proper exception when str.rsplit is passed a regex and clarify the docs #62035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

BUG: raise a proper exception when str.rsplit is passed a regex and clarify the docs #62035

wants to merge 2 commits into from

Conversation

@hamdanal
Copy link

@hamdanal hamdanal commented Aug 3, 2025

Noticed this while working on pandas-dev/pandas-stubs#1278. rsplit doesn't accept regular expressions but was silently accepting them and producing bad results. I added a check on the type of the input and updated the documentation.
mroeschke
mroeschke reviewed Aug 4, 2025
View reviewed changes
pandas/core/strings/accessor.py
Comment on lines +946 to +948
if pat is not None and not isinstance(pat, str):
msg = f"expected a string object, not {type(pat).__name__}"
raise TypeError(msg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put this in _str_rsplit for each implementation?
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure no problem but this exact error is raised here for the other methods so I was following the same process. See for example find.
@mroeschke mroeschke added the Strings String extension data type and string data label Aug 4, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2025

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.
@github-actions github-actions bot added the Stale label Sep 4, 2025
@mroeschke
Copy link
Member

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.
@mroeschke mroeschke closed this Oct 5, 2025
@hamdanal
Copy link
Author

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.

Thanks @mroeschke. There were no failing tests and I just merged main. Could you please reopen?

Regarding the unresolved review comment above, could you explain why you want the exception to be raised for each implementation instead of in StringAccessor.rsplit? This exception is not implementation-dependent but is core to the rsplit design. Many other similar exceptions are implemented the same way like in StringAccessor.find, StringAccessor.rfind, StringAccessor.index, StringAccessor.rindex just to name a few. If you feel strongly about this I can look into moving the logic into each implementation but it looks much easier to stick to the existing practice instead.
@mroeschke mroeschke reopened this Oct 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stale Strings String extension data type and string data

2 participants

@hamdanal @mroeschke