-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: SparseArray numeric ops misc fixes #12910
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
@@ -59,7 +59,12 @@ def wrapper(self, other): | |||
|
|||
|
|||
def _sparse_array_op(left, right, op, name): | |||
if np.isnan(left.fill_value): | |||
if (np.isnan(left.fill_value) and np.isnan(right.fill_value) and |
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.
so this begs the question of why we don't just always pass the fill values to the ufuncs (e.g. sparse_sub
etc), which can then decide (based on np.isnan
or (isnull
better)) on left and/or rhs whether to use it?
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.
Yes removing nanop
simplifies the logic. Because data is sparse, it shouldn't affect to performance in most cases.
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.
do you want to do that in this PR?
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.
Yes, let me try.
whoosh you blew away lots of code. must have been work-arounds built in there for maybe an old cython or something. Just want to be sure (since tests were removed) everything still working (as may not be testing some of the numeric ops as much, though did see you added some tests) |
The removed test calls |
thanks! |
git diff upstream/master | flake8 --diff
Fixed following 3 issues occurred on the current master.
1. addition ignores rhs
fill_value
2. mod raises
AttributeError
3. pow outputs incorrect result wiht
1.0 ** np.nan