-
Notifications
You must be signed in to change notification settings - Fork 87
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
ci: fix oldest-supported-dependencies test workflow #2783
Conversation
Codecov Report
Additional details and impacted files
|
84320c4
to
4da1997
Compare
28f8055
to
6c708c6
Compare
bf96837
to
aeeb3f6
Compare
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.
This PR fixes both the test infrastructure and also the bug for old versions of NumPy. I strongly approve of the use of a NUMPY_HAS_NEP_50
instead of finding a solution that works for both old and new NumPy, so that we know what to remove when our minimum NumPy does get to be 1.24 or above.
I think this is ready to merge!
It turns out that our CI wasn't properly testing the package combinations that we'd asked it to — there were subsequent install steps that overwrote the intended variations. This means that #2767 was not properly tested against older versions of NumPy, leading to a bug.
Although it's possible to use multiple install steps, it best reflects our intentions if we use a single install command.
In future, we probably just want to have a single
requirements.txt
file, and a separateconstraints.txt
that pins these dependency versions on our lower-bound configuration. However, we don't currently test all dependencies at the lower bound. This PR only runs core tests (NumPy + PyArrow) in the 3.8 tests.Fixes #2784