Skip to content

fix: FindNumPy should not call FindPythonLibs#958

Merged
henryiii merged 1 commit intomainfrom
henryiii-patch-1
Apr 27, 2023
Merged

fix: FindNumPy should not call FindPythonLibs#958
henryiii merged 1 commit intomainfrom
henryiii-patch-1

Conversation

@henryiii
Copy link
Copy Markdown
Contributor

This should not be necessary. It should already be found, and even if not, PYTHON_EXECUTABLE is set and PYTHON_INCLUDE_DIR isn't important. I think this is causing #957, since you can't run FindPythonLibs REQUIRED on manylinux.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 26, 2023

Codecov Report

Merging #958 (a6ba853) into main (82d905e) will increase coverage by 5.72%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #958      +/-   ##
==========================================
+ Coverage   80.57%   86.29%   +5.72%     
==========================================
  Files          31       31              
  Lines        1560     1562       +2     
  Branches      283      343      +60     
==========================================
+ Hits         1257     1348      +91     
+ Misses        231      152      -79     
+ Partials       72       62      -10     

see 8 files with indirect coverage changes

@tommy-waltmann
Copy link
Copy Markdown

I have tested this on the manylinux image for myself and the build now succeeds, however there is still a message being printed which reads:

-- Could NOT find PythonLibs (missing: PYTHON_LIBRARIES) (found version "3.9.16")

While this is technically still true because there is no libpython.so in the manylinux images, it could mislead users into thinking there is something wrong with the cmake configuration when everything is fine.

@henryiii
Copy link
Copy Markdown
Contributor Author

That warning is correct; on manylinux, there are no Python libs. Your users are not likely to be building on manylinux (since it's only for making wheels), so will probably not get that warning - most Python distributions do include the libs. The root problem I expect is PythonExtensions is calling FindPythonLibs, which triggers that warning. Maybe we can see if we can fix that.

@henryiii henryiii changed the title fix: FindNumPy should call FindPythonLibs fix: FindNumPy should not call FindPythonLibs Apr 27, 2023
@henryiii henryiii merged commit 7c9d8d2 into main Apr 27, 2023
@henryiii henryiii deleted the henryiii-patch-1 branch April 27, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants