CI and Build: Updated setup.py and continuous integration configuration #298
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Apr 5, 2024
|
This PR also fixes support of Ready for review. |
|
Indeed a tough one ... |
kif
reviewed
Apr 12, 2024
kif
reviewed
Apr 12, 2024
| @@ -122,8 +122,7 @@ def testFcidecomp(self): | |||
| self.assertTrue(data.shape[0] == 60, "Incorrect shape") | |||
| self.assertTrue(data.shape[1] == 30, "Incorrect shape") | |||
| self.assertTrue(data.dtype == expected_data.dtype, "Incorrect type") | |||
| self.assertTrue(numpy.alltrue(data == expected_data), | |||
| "Incorrect values read") | |||
| self.assertTrue(numpy.all(data == expected_data), "Incorrect values read") | |||
There was a problem hiding this comment.
On the one hand, you specify numpy2 is not supported (in .github/workflows/release.yml), and here you make it compatible ...
Is there a list of plugins which are not yet numpy2-compatible ? Such a list should be made and updated when the compatibility is validated.
There was a problem hiding this comment.
Tests are run twice each time:
- a first time with the latest versions of
h5py&numpy, so as of today, numpy2 if installed with--pre - a second time with the "oldest supported" version of h5py for the given version of Python, and in this case I added
numpy<2because those versions ofh5pydo not support numpy2.
The numpy<2 in the CI configuration files are for the later case.
|
Beside the 2 comments LGTM |
|
Thanks for the review! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims at updating/simplifying the
setuptoolsand CI configs and removing unnecessary code.The main breaking change is the removal of
setup.py buildcommand line arguments that was deprecated in favor of environment variables.Another change is the requirement of
setuptools>62.4.0andpy-cpuinfo==9.0.0, but that won't be visible when installing withpip.I would include this in a major release.
Details of the changes:
setuptools>62.4.0: allow usingpyproject.tomland removing fallbacks todistutilssetup.pytopyproject.tomlci/info_platform.py[dev]to[doc]and use it instead ofrequirements.txtto avoid duplicating the list of dependenciescpuinfo.py: This is only needed when building withpython setup.py build(as issetuptools). In this case, a log indicates to installpy-cpuinfo.setup.py buildcommand line arguments since this is a deprecated feature ofsetuptools.setup.py