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
MAINT: Address tuple indexing and warnings #8944
Conversation
sl[axis] = slice(-(N - 1) // 2, None) | ||
Y[sl] = X[sl] | ||
Y[tuple(sl)] = X[tuple(sl)] |
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.
Probably faster to not call tuple twice 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.
I think for the code that is executed here the speed difference will be negligible, and I'm not sure it would help readability to assign it to a temp var.
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.
Also I suppose numpy was calling tuple
twice before anyway internally, so this is not really any slower.
assert_equal(log1p(c(np.nan, 1)), c(np.nan, np.nan)) | ||
assert_equal(log1p(c(np.nan, np.nan)), c(np.nan, np.nan)) | ||
with suppress_warnings() as sup: | ||
sup.filter(RuntimeWarning, "invalid value encountered in multiply") |
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.
This doesn't seem to have an associated change in the warning list
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.
The PR that was merged before had ignore
in the list, which I think caused this (and potentially other) warning(s) to be ignored
LGTM and all green, so in it goes. Thanks @larsoner, @eric-wieser |
Thanks @larsoner for picking up the other half of numpy/numpy#9686 for me! |
With this PR, I get no warnings in
runtests.py
. We'll see if the CIs agree.I also fixed a shadowing of the builtin
sorted
.