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

chore: port to numpy 2.0 #60

Merged
merged 20 commits into from
Aug 21, 2024
Merged

chore: port to numpy 2.0 #60

merged 20 commits into from
Aug 21, 2024

Conversation

ianna
Copy link
Collaborator

@ianna ianna commented Aug 5, 2024

fixes issue #56

Copy link
Collaborator Author

@ianna ianna left a comment

Choose a reason for hiding this comment

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

@ohrechykha - the tests with Numpy version 1 are failing because numpy.array_api is experimental. Please, add the ignore statement as below.

tests/test_spec_elementwise_functions.py Outdated Show resolved Hide resolved
@ianna
Copy link
Collaborator Author

ianna commented Aug 8, 2024

@ohrechykha - looks good! There are two more issues left. You can check them by clicking on "Details". One is the lower bound testing (issue #61). It should be fixed in a separate PR, I think. And another is related to the differences in Numpy versions (asarray) that should be fixed in this PR.

@ohrechykha
Copy link
Collaborator

I'm unsure what's wrong: the idea was that defining _wrapper in floor and ceil functions is unnecessary (because I defined it in ceil but forgot to add it to floor and nothing happened). After seeing the failures I've concluded this was wrong and returned them. The ubuntu tests still fail with the same errors, while on Windows I haven't gotten any errors regardless of whether there's a wrapper function in ceil and floor, meaning it could likely be removed.

@ianna
Copy link
Collaborator Author

ianna commented Aug 8, 2024

I'm unsure what's wrong: the idea was that defining _wrapper in floor and ceil functions is unnecessary (because I defined it in ceil but forgot to add it to floor and nothing happened). After seeing the failures I've concluded this was wrong and returned them. The ubuntu tests still fail with the same errors, while on Windows I haven't gotten any errors regardless of whether there's a wrapper function in ceil and floor, meaning it could likely be removed.

The tests in the CI pass :-)

@ohrechykha
Copy link
Collaborator

ohrechykha commented Aug 12, 2024

Oh well. I've made it work for numpy 1.22.3, while git is testing it on 1.22.0...

Edit: nevermind this still fails at 1.22.0

@ohrechykha ohrechykha requested a review from jpivarski August 14, 2024 13:33
@jpivarski jpivarski marked this pull request as ready for review August 14, 2024 14:02
@hmaarrfk
Copy link

is numpy 1.22 a version you intend on supporting? Numpy recommends not supporting lower than 1.24 in june of 2024.

https://numpy.org/neps/nep-0029-deprecation_policy.html#support-table

Just trying to help make these chores easier!

@jpivarski
Copy link
Member

NumPy 1.22 was chosen as a minimum for Awkward Array, based on functionality. (Something was added in NumPy 1.22 that Awkward requires; no tighter requirements were needed after that.) We can get on the train of always-moving dependency windows, but we just haven't yet.

If ragged needs something that NumPy 1.22 doesn't provide—that is, if it works in a later version of NumPy but not 1.22—then let's just increase the minimum NumPy version as needed, especially since it agrees with NumPy's own schedule of support.

In principle, we should be moving our version constraints according to the SPEC 0 schedule; we just haven't started that yet.

@ianna
Copy link
Collaborator Author

ianna commented Aug 19, 2024

@jpivarski and @ohrechykha - it looks like Numpy 2.1.0 went back to return the same dtype as its elements dtype.

@ianna
Copy link
Collaborator Author

ianna commented Aug 19, 2024

@jpivarski - Please, review when you have time. Thanks!

@ianna ianna merged commit a83deb4 into main Aug 21, 2024
12 checks passed
@ianna ianna deleted the oleksii/ragged_port_to_numpy_2 branch August 21, 2024 14:38
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.

4 participants