-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
API: Handle pow & rpow special cases #30097
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
Conversation
def test_rpow_special(value): | ||
result = value ** pd.NA | ||
assert result == 1 | ||
if not isinstance(value, (np.float_, np.bool_, np.int_)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somebody (Cython?) is converting these to Python scalars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have you tried with/without numexpr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can numexpr be involved in this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont know, but if we're brainstorming things that can do surprising conversions, it comes to minid
The NA scalar itself is currently documented in missing_data.rst. I would maybe add it there? |
Didn't see that section. Moved the note there. We will still need something in reference so that :class:`NA` points somewhere. |
This is blocking #29964, so it'd be nice to merge this in soon if possible. |
Addressed those, thanks for catching that. |
|
||
@pytest.mark.parametrize( | ||
"value", [1, 1.0, True, np.bool_(True), np.int_(1), np.float_(1)] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corner case to check -0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch.
In [4]: -1 ** np.nan
Out[4]: -1.0
In [5]: np.nan ** -0
Out[5]: 1.0
Will match the NumPy behavior here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General question - is this behavior defined or standardized somewhere?
@@ -69,6 +72,49 @@ def test_comparison_ops(): | |||
assert (other <= NA) is NA | |||
|
|||
|
|||
@pytest.mark.parametrize( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are fixtures defined for zero / one values in pandas/tests/indexing/conftest.py - can these be combined with that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems to include arrays. We just want a scalar here I think.
It's part of some IEEE spec, but I haven't tracked down the source. Both Python and NumPy use this behavior for NaN. For NA, I think it aligns with the principles behind Kleene logic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying comments. Good on all points from my perspective
thanks @TomAugspurger |
Closes #29997
@jorisvandenbossche do you have thoughts on where to document this behavior? I added a bit to
reference/arrays.rst
for now, but perhaps we'll eventually have auser_guide/scalar_na.rst
?