Make get_loc with nan for FloatIndex consistent with other index types by phofl · Pull Request #39382 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation21 Commits10 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
- tests added / passed
- Ensure all linting tests pass, see here for how to run them
- whatsnew entry
Have not added a whatsnew because I am not sure if 2 is expected here. You could also make a point for retruning nan. But to handle these cases consistently, we have to check for method here
Related to indexing on series/frames, not to indexes themselves
label
i think test failures should be fixed by merging master
Get loc nan Make get_loc with nan for FloatIndex consistent with other index types
| @@ -57,7 +57,10 @@ cdef class {{name}}Engine(IndexEngine): |
|---|
| with warnings.catch_warnings(): |
| # e.g. if values is float64 and `val` is a str, suppress warning |
| warnings.filterwarnings("ignore", category=FutureWarning) |
| indexer = values == val |
| if cmath.isnan(val): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably should use tempita to only do this check for FloatEngine
might be able to use something in the cnp namespace for the isnan check
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do this only for float, test_slice_locs_na raises from /indexes/test_base.py
Actually we don't need imports I think. Changed it
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
util.is_nan might work
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah thx.
| @@ -57,7 +57,10 @@ cdef class {{name}}Engine(IndexEngine): |
|---|
| with warnings.catch_warnings(): |
| # e.g. if values is float64 and `val` is a str, suppress warning |
| warnings.filterwarnings("ignore", category=FutureWarning) |
| indexer = values == val |
| if val != val: |
| indexer = values != values |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something like
{{ if dtype in ["float64", "float32"]}}
if val != val:
indexer = np.isnan(values)
else:
indexer = values == val
{{else}}
indexer = values == val
{{endif}}
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the same thing yesterday, this caused the test to fail. Weird...
@phofl can you merge master. ok with fixing this here or in followup to raise.
This is raising now if nan is not in Index
very nice!
this may close other issues (hard to search for but looking for indexing and nan)
Labels
Related to indexing on series/frames, not to indexes themselves