-
Notifications
You must be signed in to change notification settings - Fork 58
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
base: main
Are you sure you want to change the base?
Conversation
c246439
to
25a09c4
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
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.
291d1a9
to
164b00d
Compare
164b00d
to
251f83d
Compare
if numpy_version >= (2, 0, 0): | ||
assert x.dtype == np.int64 | ||
else: | ||
assert x.dtype == np.int32 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
"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'", |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
- numpy claims that ABI is backward compatible from 2 to 1.x.x
- All of the tests are passing in the RMS 14.2 workflow in this PR which runs against
numpy==1.24.3
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.