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

BLD: Build against numpy >= 2.0.0rc1 #1233

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mferrera
Copy link
Collaborator

@mferrera mferrera commented Aug 23, 2024

Resolves #1209

This unpins numpy<2 and builds the SWIG C library against numpy >= 2.0.0rc1. Versions of numpy major 2 are backward compatible with numpy 1 with respect to the C api. Hence we should not see issues in environments where numpy<2 may be maintained for years to come, i.e. RMS environments.

The numpy SWIG bindings committed in this repository are not out-of-date in a way that would break compatibility. The last update to them in numpy's repository was two years ago from the time of this commit.

As a consequence of supporting numpy>=2 this PR deprecates support for Python 3.8, which hits EOL on October 2024.

@codecov-commenter
Copy link

codecov-commenter commented Aug 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.95%. Comparing base (0375fed) to head (251f83d).
Report is 27 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1233      +/-   ##
==========================================
+ Coverage   80.02%   80.95%   +0.92%     
==========================================
  Files          98       92       -6     
  Lines       13680    12175    -1505     
  Branches     2203     1981     -222     
==========================================
- Hits        10948     9856    -1092     
+ Misses       1999     1664     -335     
+ Partials      733      655      -78     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This unpins numpy<2 and builds the SWIG C library against numpy >=
2.0.0rc1. Versions of numpy major 2 are backward compatible with numpy
1. Hence we should not see issues in environment where numpy<2 may be
maintained for years to come, i.e. RMS environments.

The numpy SWIG bindings committed in this repository are not out-of-date
in a way that would break compatibility. The last update to them in
numpy's repository was two years ago from the time of this commit.
@mferrera mferrera force-pushed the unpin-numpy-2 branch 3 times, most recently from 291d1a9 to 164b00d Compare August 23, 2024 13:07
@mferrera mferrera self-assigned this Aug 23, 2024
@mferrera mferrera marked this pull request as ready for review August 23, 2024 13:18
@mferrera mferrera requested a review from jcrivenaes August 23, 2024 13:23
Comment on lines +121 to +124
if numpy_version >= (2, 0, 0):
assert x.dtype == np.int64
else:
assert x.dtype == np.int32
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should probably take another look into this and if there are implications for file sizes

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this indicate some problems with backward compatibility?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to test this more

Comment on lines -6 to -10
"numpy==1.19.2; python_version == '3.8'",
"numpy==1.19.5; python_version == '3.9'",
"numpy==1.21.6; python_version == '3.10'",
"numpy==1.23.5; python_version == '3.11'",
"numpy==1.26.2; python_version == '3.12'",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not think this should be replaced without proper testing. The reason is that numpy version for RMS is pinned, and to avoid ABI version issues we should compile on the oldest version of numpy that is supported by a given python version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree that we will need to do some thorough testing. A few good indicators so far:

@mferrera mferrera added the blocked Blocked by something else label Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Blocked by something else
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unpin numpy <2
3 participants