-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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.
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
DEPR (string): non-bool na for obj.str.contains #59615
DEPR (string): non-bool na for obj.str.contains #59615
Conversation
I think startswith/endswith have the same issue? If so, can you add the same deprecation message there as well?
Yeah, we should start a 2.3 whatsnew file. We should add it to main, though (not in the backport PR), so things can be added to that file in PRs like this one. |
4eb3bed
to
c53f154
Compare
Sure, updated. |
Actually, you could maybe move the warning to core/string/accessors.py, then you don't have to repeat it for the various implementations, and then also ArrowDtype(string) will get it (assuming we want to be consistent and also deprecate it for ArrowDtype, and not only for StringDtype?) |
Of course, that one already errors :) |
Because it already errors for the ArrowEA version (but with a pyarrow exception), patching it in the StringAccessor method would be bulky. I think the thing to do is wait until the deprecation is enforced, then do the validation in the StringAccessor method and get consistent exception/message. |
c53f154
to
1403c87
Compare
Sounds good! |
Co-authored-by: Joris Van den Bossche <[email protected]>
Co-authored-by: Joris Van den Bossche <[email protected]>
na
validation #59561 (Replace xxxx with the GitHub issue number)doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.No 2.3 whatsnew file yet. Not sure if the plan is to introduce one in #59513