Skip to content

CLN: Enforce deprecation of not validating na argument to string methods #62399

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

Open

rhshadrach wants to merge 1 commit into pandas-dev:main

from rhshadrach:cln_enforce_na_validation

Open

CLN: Enforce deprecation of not validating na argument to string methods #62399

rhshadrach wants to merge 1 commit into pandas-dev:main from rhshadrach:cln_enforce_na_validation

+52 −70

Conversation

Copy link

Member

@rhshadrach rhshadrach commented Sep 21, 2025

closes #xxxx (Replace xxxx with the GitHub issue number)
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

The default of na in these methods was changed from None to no_default in #59616; I am assuming we want to stick with no_default.


 CLN: Enforce deprecation of not validating argument

d593ffa

@rhshadrach rhshadrach added Error Reporting Strings Clean labels

jorisvandenbossche

jorisvandenbossche reviewed

View reviewed changes

pandas/util/_validators.py

return

if isna(value):

return

raise ValueError(f"{name} must be a valid NA value; got {value}")

Copy link

Member

@jorisvandenbossche jorisvandenbossche Sep 21, 2025

Choose a reason for hiding this comment

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

Suggested change

raise ValueError(f"{name} must be a valid NA value; got {value}")

raise ValueError(f"{name} must be a bool (True/False) or a valid NA value; got {value}")

Copy link

Member

@jorisvandenbossche jorisvandenbossche Sep 21, 2025

Choose a reason for hiding this comment

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

BTW, I don't know if there is a case for which you are foreseeing allow_no_default and allow_bool to be used, but if they would actually be used, then the error message would also have to be updated to follow that.

(so for now could also simplify things leaving out the keywords?)

Copy link

Member Author

@rhshadrach rhshadrach Sep 22, 2025 •

edited

Loading

Choose a reason for hiding this comment

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

How would you feel about ...must be a valid value; got {value}?

Copy link

Member

@jorisvandenbossche jorisvandenbossche Sep 22, 2025

Choose a reason for hiding this comment

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

I personally think it is useful to give an indication about what a valid value is (or say "a valid value for dtype ..", so then if it is bool, then it is clearer that it should be True/False or a missing value (for nullable bool)?)

pandas/tests/strings/test_find_replace.py Show resolved Hide resolved

@rhshadrach rhshadrach changed the title ~~(削除) CLN: Enforce deprecation of not validating argument (削除ここまで)~~ (追記) CLN: Enforce deprecation of not validating na argument to string methods (追記ここまで)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Clean Error Reporting Strings

2 participants

@rhshadrach @jorisvandenbossche