Skip to content
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

feat: adding support for numpy.real, imag, round, angle, #3053

Conversation

tcawlfield
Copy link
Collaborator

Closes #2917

This issue does not mention round explicitly. and it does mention around which I have not duplicated yet.
I could additionally implement numpy.fix and real_if_close.

There is an error in my attempt to support the out=<array> optional argument -- see discussion.

Fixing bugs in TypeTracer.angle.
Simplifying test_complex_ops.
Adding tests of TypeTracer real, imag, angle.
@tcawlfield tcawlfield self-assigned this Mar 15, 2024
@tcawlfield
Copy link
Collaborator Author

Are there automatic code formatters and formatting checks? I guess I'll find out! Oh, I did find out and there are indeed.

@ianna
Copy link
Collaborator

ianna commented Mar 15, 2024

Are there automatic code formatters and formatting checks? I guess I'll find out! Oh, I did find out and there are indeed.

Indeed, they are. You can run pre-commit run --all and fix the warnings before pushing then you will not need to pull the automatically updated branch from remote.

@jpivarski
Copy link
Member

We don't yet have automatic code formatting on C++, but black ruff auto-formats Python. (And some other things in pre-commit format YAML and TOML.) We've talked about formatting C++ (#34, #1865, #2902), but never converged.

I agree with @ianna that it's more convenient to run pre-commit locally, since formatting changes can automatically write new commits to the PR, and then if you commit on your local clone, you've got a conflict to manage.

Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

This implements real, imag, angle, and round as NEP-18 overloads, since they're not ufuncs like np.floor:

>>> np.real
<function real at 0x70646be60d70>
>>> np.imag
<function imag at 0x70646be613f0>
>>> np.angle
<function angle at 0x7063e9b4ac30>
>>> np.round
<function round at 0x70646bfd3eb0>
>>> np.floor
<ufunc 'floor'>

Overall, it looks good; I just have a few inline requested changes.

src/awkward/_nplikes/typetracer.py Outdated Show resolved Hide resolved
src/awkward/_nplikes/typetracer.py Outdated Show resolved Hide resolved
src/awkward/operations/ak_real_imag_angle.py Outdated Show resolved Hide resolved
As per Jim's recommendation, for real, imag, angle.
@jpivarski jpivarski changed the title Adding support for numpy.real, imag, round, angle, feat: adding support for numpy.real, imag, round, angle, Mar 18, 2024
@jpivarski
Copy link
Member

Let me know when you want me to re-review this!

@jpivarski
Copy link
Member

Great, thanks! This one is done and I'll merge it.

@jpivarski jpivarski enabled auto-merge (squash) March 20, 2024 22:12
@jpivarski jpivarski merged commit a6444b0 into main Mar 20, 2024
38 checks passed
@jpivarski jpivarski deleted the 2917-npreal-npimag-npround-nparound-cant-take-an-awkward-array-as-input branch March 20, 2024 22:28
@jpivarski
Copy link
Member

@all-contributors please add @tcawlfield for code

Copy link
Contributor

@jpivarski

I've put up a pull request to add @tcawlfield! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

np.real, np.imag, np.round, np.around can't take an Awkward Array as input
4 participants