Add numpy dependency to python wrapper. #1935
Conversation
- Additional dependencies required for the build environment Numpy must be installed in the python environment where the build is being performed in order to build. - Automatic installation of dependencies like a normal python package When installing this package, if numpy does not exist in the python environment, it will be installed automatically. Signed-off-by: HyeongSeok Kim <[email protected]>
|
Can I ask what's the rationale for adding a dependency on numpy? Technically someone could use any other libraries that have a compatible numpy like array types. |
| @@ -62,6 +62,10 @@ jobs: | |||
| CIBW_SKIP: "*-win32 *_i686" | |||
| CIBW_TEST_SKIP: "*-macosx_universal2:arm64" | |||
| CIBW_ENVIRONMENT: OPENEXR_RELEASE_CANDIDATE_TAG="${{ github.ref_name }}" | |||
| CIBW_BEFORE_BUILD: | | |||
| pip install tomli | |||
There was a problem hiding this comment.
This is not needed. The build frontend (pip, build, etc) will automatically install the dependencies specified in the pyproject's build-system.requires section.
pyproject.toml
Outdated
| @@ -2,7 +2,7 @@ | |||
| # Copyright (c) Contributors to the OpenEXR Project. | |||
|
|
|||
| [build-system] | |||
| requires = ["scikit-build-core==0.10.7", "pybind11"] | |||
| requires = ["scikit-build-core==0.10.7", "pybind11", "numpy"] | |||
There was a problem hiding this comment.
Numpy is not needed at build time. Pybind11 doesn't need numpy and will not use numpy.
|
Actually, never mind we indeed need to add it as a runtime dependency because But it only needs
https://pybind11.readthedocs.io/en/stable/advanced/pycpp/numpy.html#arrays But still, my comment about the build time dependency is still valid. We don't need numpy to build openexr. |
|
But I think I'll try to build locally and test locally, because I'd like to get a better traceback/backtrace. |
…e numpy in build requires - python-wheels-*yml: When testing on other PRs, I found that dependencies were not properly installed for each build case, so I added this code. However, this PR is for adding numpy dependencies, so it is not needed as @JeanChristopheMorinPerso suggested. - pyproject.toml: If you are going to do any additional work with the build artifacts, like generating stubs, you will need numpy in your build environment, but it is not needed at the moment. I agree with the review by @JeanChristopheMorinPerso. Signed-off-by: HyeongSeok Kim <[email protected]>
@JeanChristopheMorinPerso, thank you for your review. I agree with what you said and have modified the code. |
cary-ilm
merged commit 89fd37e
into
AcademySoftwareFoundation:main
There was an issue with the existing PR, so I'm submitting a separate PR that resolves the Numpy dependency issue.
Modify pyproject.toml.
Add numpy to package dependencies. When installing this package with pip, numpy will be installed additionally if it is not in the Python environment.
Modify python-related workflows.
Modify each python workflow to explicitly install the dependencies (pyproject.toml) required for the build.
close #1919