Info about inconsistent detection of Python version between pybind11 … #1093
Conversation
This file contains hidden or 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
jagerman
reviewed
Sep 19, 2017
docs/faq.rst
Outdated
| are not consistent with detection of Python version in pybind11. If several versions of Python are installed | ||
| in the Linux system (i.e. 2.7 and 3.5) the pybind11 defaults to 3.5, while CMake usually defaults to 2.7. | ||
| The reason for this design decision is unreliable and buggy behavior of ``find_package(PythonInterp)`` and ``find_package(PythonLibs)`` | ||
| which are among the worst-designed parts of CMake. We don't want to mimick this behavior in pybind11 in any way. |
There was a problem hiding this comment.
While I don't disagree with this statement, I do find the language a little strong for putting in the main documentation. Here's a suggestion for these first two paragraphs (feel free to edit):
The functions ``find_package(PythonInterp)`` and ``find_package(PythonLibs)`` provided by CMake
for Python version detection are not used by pybind11 due to unreliability and limitations that make
them unsuitable for pybind11's needs. Instead pybind provides its own, more reliable Python detection
CMake code. Conflicts can arise, however, when using pybind11 in a project that *also* uses the CMake
Python detection.
Consider the following CMake code when run on a system with both Python 2.7 and 3.6 installed:
jagerman
reviewed
Sep 19, 2017
docs/faq.rst
Outdated
| find_package(PythonLibs) | ||
| find_package(pybind11) | ||
|
|
||
| It will detect Python 2.7 and pybind11 will peek it as well. |
There was a problem hiding this comment.
Did you mean "peek" -> "pick"?
jagerman
reviewed
Sep 19, 2017
docs/faq.rst
Outdated
| will detect Python 3.5 for pybind11 and may crash on ``find_package(PythonLibs)`` afterwards. | ||
|
|
||
| It is advised to avoid using ``find_package(PythonInterp)`` and ``find_package(PythonLibs)`` from CMake and rely | ||
| on pybind11 in detecting Python version. if this is not possible CMake machinery should be called *before* including pybind11. |
There was a problem hiding this comment.
"... and rely on pybind11's more robust Python version detection instead."
Also the first "i" in "if this is not possible..." should be capitalized.
Too aggressive wording removed.
|
Merged, thanks! |
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.
Info about inconsistent detection of Python version between pybind11 and CMake in FAQ