Skip to content

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

Merged
merged 4 commits into from
Jul 26, 2025
Merged

Conversation

juntyr
Copy link
Contributor

@juntyr juntyr commented Jul 23, 2025

Supersedes #118

Progress on #111

  • implements the sign, signbit, and copysign ufuncs
  • extends unary unfunc tests
  • adds extra const annotations to the ops implementations

@juntyr
Copy link
Contributor Author

juntyr commented Jul 23, 2025

@SwayamInSync A manual rebase was easier for me, this PR should now be good to go

@juntyr juntyr mentioned this pull request Jul 21, 2025
11 tasks
@juntyr
Copy link
Contributor Author

juntyr commented Jul 25, 2025

@SwayamInSync @ngoldbaum Is there anything left to do so that this PR can be merged?

@SwayamInSync
Copy link
Collaborator

@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.

@juntyr
Copy link
Contributor Author

juntyr commented Jul 26, 2025

@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?

Copy link
Collaborator

@SwayamInSync SwayamInSync left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks LGTM

@SwayamInSync SwayamInSync merged commit 0944dfa into numpy:main Jul 26, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants