Skip to content
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

Add "N / A" to STR_NA_VALUES #59150

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

NickCrews
Copy link
Contributor

This is a value that I just came across in some data that is in the spirit of the other NA value. I don't think this will be a false positive any more so than some of the other values in this list.

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature. A test here would just be duplicating the implementation.
  • 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. I don't think this deserves an entry?

This is a value that I just came across in some data that is in the spirit of the other NA value.
I don't think this will be a false positive any more so than some of the other values in this list.
@NickCrews
Copy link
Contributor Author

PS, I think it is usually bad taste for a test to exactly copy the contents of the implementation like this. Can I just make it so we reference STR_NA_VALUES directly in the test, and remove _NA_VALUES from that test?

@WillAyd
Copy link
Member

WillAyd commented Jul 1, 2024

Thanks for the PR but its not really a feasible goal for us to track down every NA value a user could come across, and dealing with whitespace like this is not something I see of being value added to pandas. I think you may just need to handle this in your own process

@NickCrews
Copy link
Contributor Author

That is definitely true, I already have a selection of additional values I check for. This is just the most egregious one that I thought would.be applicable.for everyone.

Even if we don't get to perfection, I think this PR still improves the situation and is a good contribution. Are you worried that slowly this list will balloon to 100 different examples? Trying to figure out if there is some mitigation o can do to address your concern. Thank you, I appreciate your time and effort!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants