BUG: argmin/max of object dtype with NULLs, fixes #6032#6236
BUG: argmin/max of object dtype with NULLs, fixes #6032#6236charris merged 1 commit intonumpy:masterfrom
Conversation
There was a problem hiding this comment.
This looks equivalent to i == n.
There was a problem hiding this comment.
They way figured, if all entries are NULL, then at the end i == n and mp == NULL, but if the last entry is the first non-NULL, then at the end i == n but mp != NULL. It is probably a good idea to add test cases with both these situations.
jaimefrio
force-pushed
the
argminmax_object
branch
2 times, most recently
from
8198e4e to
c606498
Compare
|
I just realized we are happily assuming that the object comparison will never fail, which we know is not always the case, especially in Python 3. Added a check to exit on first failure. And I also followed your suggestion for skipping over the NULLs, although I simplified it further by not special casing the first item. |
|
LGTM. I'd probably have bailed early on all NULL, but there is always more than one way ;) Is there any error return from these function that might be appropriate for a comparison error? Oh, and IIRC, richcompare is available in all supported Python versions and falls back to cmp in Python 2 if only cmp is available. I wonder if we should stop using the fallback compare? Might be something we need to pass by the list, but it might also provide more consistent behavior for Python 2 and 3. |
jaimefrio
force-pushed
the
argminmax_object
branch
from
c606498 to
c3c5509
Compare
|
Yup, 2.6 has it, and so does my latest commit. |
|
Should probably mention in release notes under |
BUG: argmin/max of object dtype with NULLs, fixes #6032
|
Thanks Jaime. I'm going to update the 1.11 notes anyway and will mention this. |
2 participants
No description provided.