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

fix: ak.from_numpy should fail on zero-dimensional arrays. #3161

Conversation

tcawlfield
Copy link
Collaborator

At this point, the first tests, ak.Array(), was already passing without modifications.
But the second test was not -- ak.from_numpy().
Fix is in _layout.py. No policy options are passed to _layout.from_arraylib().

At this point, the first tests, ak.Array(), was already passing without modifications.
But the second test was not -- ak.from_numpy().
Fix is in _layout.py. No policy options are passed to _layout.from_arraylib().
@tcawlfield
Copy link
Collaborator Author

Ugh this was too naive. Breaks a bunch of other tests.

@tcawlfield tcawlfield changed the title policy: ak.from_numpy should fail on zero-dimensional arrays. fix: ak.from_numpy should fail on zero-dimensional arrays. Jun 20, 2024
From from_numpy, from_cupy, from_jax, and from_dlpack
To from_arraylib
@tcawlfield
Copy link
Collaborator Author

Ugh this was too naive. Breaks a bunch of other tests.

Okay, I fixed the problems by adding a primitive_policy kwarg to _layout.from_arraylib and upstream functions.
I'm not sure now why all the CI tests are failing. They pass on my machine, and the reasons seem to relate to C++ issues outside the scope of my change here.

This change ended up being noisier than I would have liked, but I don't see any other way to get the desired policy.

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.

If from_arraylib were a public function, I'd want to think about the consequences of adding a primitive_policy argument. As an internal function, we can always reconsider it's interface at a later date, and this interface looks good enough for now. But the defaults seem to be inconsistent, and that would make it hard to think about.

Understanding the test failures will be important, since they're telling us that behavior is changing—probably not in ways that we want.

src/awkward/_layout.py Show resolved Hide resolved
@tcawlfield tcawlfield merged commit db6cece into main Jun 24, 2024
39 checks passed
@tcawlfield tcawlfield deleted the 1057-akarray-and-akfrom_numpy-should-not-accept-zero-dimensional-arrays branch June 24, 2024 21:46
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.

ak.Array and ak.from_numpy should not accept zero-dimensional arrays
3 participants