-
-
Notifications
You must be signed in to change notification settings - Fork 9
Implement copysign, sign, and signbit ufuncs #122
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
@SwayamInSync A manual rebase was easier for me, this PR should now be good to go |
@SwayamInSync @ngoldbaum Is there anything left to do so that this PR can be merged? |
Yes just a small edit, in the earlier review I mentioned a variable shadowing issue, can you please fix that, it is not causing any error right now because inside loop statements the lists gets interpreted first. So it captures the original value. But this is confusing and error-prone. |
I'm unfortunately unable to find that review, could you please link it for me? |
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 LGTM
I'm glad you two are handling reviews between each other. Please feel free to ping me if you'd like an additional eye or opinion on anything. |
Supersedes #118
Progress on #111
sign
,signbit
, andcopysign
ufuncsconst
annotations to the ops implementations