COMP: Prefer to use find_package(Python3 )#92
Closed
hjmjohnson wants to merge 8 commits intocommontk:patched-v3.6.1-2025-09-30-f4769f190from
Closed
COMP: Prefer to use find_package(Python3 )#92hjmjohnson wants to merge 8 commits intocommontk:patched-v3.6.1-2025-09-30-f4769f190from
hjmjohnson wants to merge 8 commits intocommontk:patched-v3.6.1-2025-09-30-f4769f190from
Conversation
This was referenced Jul 16, 2025
Author
|
@jamesobutler Would you please have a look at this PR? |
|
Will do. I'll review in the context of the corresponding Slicer PR and then provide approvals for the chain of PRs as they get updated. |
There was a problem hiding this comment.
Pull Request Overview
The PR updates the project's CMake configuration to use the newer find_package(Python3) API for more robust control over Python versions and ABI.
- Replace the legacy
find_package(PythonLibs)call withfind_package(Python3 COMPONENTS Interpreter Development REQUIRED) - Update
include_directoriesto use thePython3_INCLUDE_DIRSvariable
| find_package(PythonLibs REQUIRED) | ||
| include_directories("${PYTHON_INCLUDE_DIR}") | ||
| find_package(Python3 COMPONENTS Interpreter Development REQUIRED) | ||
| include_directories("${Python3_INCLUDE_DIRS}") |
There was a problem hiding this comment.
[nitpick] Instead of using the global include_directories command, link against the Python3 imported target (e.g., Python3::Python) via target_link_libraries. This automatically brings in the correct include paths and libraries, aligning with modern CMake practices.
|
|
||
| find_package(PythonLibs REQUIRED) | ||
| include_directories("${PYTHON_INCLUDE_DIR}") | ||
| find_package(Python3 COMPONENTS Interpreter Development REQUIRED) |
There was a problem hiding this comment.
[nitpick] Consider specifying a minimum Python version (for example, find_package(Python3 3.8 COMPONENTS Interpreter Development REQUIRED)) to enforce consistent ABI and feature availability across developer environments.
Suggested change
| find_package(Python3 COMPONENTS Interpreter Development REQUIRED) | |
| find_package(Python3 3.8 COMPONENTS Interpreter Development REQUIRED) |
This commit prevents crashes by handling scenarios such as: (a) object destruction after the Python interpreter has been finalized (b) object destruction after cleanup, i.e. the singleton no longer exists Any usage of a Qt enum demonstrates (a). One example that demonstrates (b) is a QTimer object which is created with a QApplication parent. PythonQt::cleanup() is called before the QApplication is completely destroyed, so the code that handles wrapping the QTimer object must handle the case when the PythonQt singleton no longer exists. Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
Add tests that check for clean cleanup and finalization in different scenarios. Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com> Co-authored-by: Pat Marion <james.patrick.marion@gmail.com>
This commit fixes a crash during PythonQt::cleanup(). While destroying the PythonQtPrivate instance, any remaining PythonQtSignalReceivers that are kept alive only by their parent object will be destroyed. The crash occurs when PythonQtSignalReceiver's destructor calls back into PythonQtPrivate::removeSignalEmitter() when the PythonQtPrivate instance is mostly destroyed. Includes test case that crashes without the fix.
This fix errors of that sort:
moc_PythonQtStdDecorators.cxx:152:25:
error: expected unqualified-id
case 4: _t->emit((*reinterpret_cast<
QObject*(*)>(_a[1])),(*reinterpret_cast< const
Add fix for mac build error related C standard lib macros to main header
By updating PythonQtPythonIncludes.h which is included in all PythonQt
headers, it is ensured the fix will be applied consistently.
PyObject_GetAttrString returns a new reference. PyDict_SetItemString does not steal a reference, so Py_DECREF should be called after PyDict_SetItemString.
…from commontk/PythonQt fork This commit partially reverts r431 (which removed unsupported files and added documentation) and consolidates consolidates historical changes developed in the `commontk/PythonQt` fork between 2011 and 2021. Summary: * Qt 5support: * Added initial and ongoing support for Qt5, including modules like `PrintSupport`, `QuickWidgets`, `Multimedia`, and `OpenGL`. * Removed Qt4 support; fixed CMake logic for building across Qt 5.3–5.9. * Introduced `pythonqt_wrap_cpp` macro and cleaned up legacy Qt macros. * Build system enhancements: * Re-enabled and expanded CMake support with configurable install paths and testing. * Removed use of `INSTALL_NAME_DIR` to support relocatable installs. * Addressed build issues across toolchains (e.g., MSVC `/bigobj` workaround, debug mode fixes). * Added missing source files and build flags. * Code quality and compatibility: * Replaced deprecated/legacy constructs (e.g., use of `nullptr`, fixed property aliasing with `name` → `objectName`). * Added support for 511 wrappers. * Resolved warnings and linkage issues with optional build flags (e.g., `PythonQt_Wrap_Qtcore`). * Testing and cleanup: * Added cleanup/finalization unit tests. * Ensured test code compiles cleanly when wrapping is partially disabled. Co-authored-by: Florian Link <5535644+florianlink@users.noreply.github.com> Co-authored-by: Matthew Woehlke <matthew.woehlke@kitware.com> Co-authored-by: Max Smolens <max.smolens@kitware.com> Co-authored-by: Pat Marion <james.patrick.marion@gmail.com> Co-authored-by: Francois Budin <francois.budin@kitware.com> Co-authored-by: Christoph Willing <chris.willing@linux.com> Co-authored-by: Stefan Dinkelacker <s.dinkelacker@dkfz-heidelberg.de> Co-authored-by: Sylvain Bernhardt <sylvain.bernhardt@smith-nephew.com>
The Python3 find_package is more robust and allows more control over ABI and version selections.
hjmjohnson
force-pushed
the
update-to-new-python3-find_package
branch
from
b46b73e to
f56ac87
Compare
hjmjohnson
changed the base branch from
patched-v3.6.1-2025-06-13-a2687828e
to
patched-v3.6.1-2025-09-30-f4769f190
|
Unsure about any of the commits other than |
Author
|
Closing. Redoing this work is easier than untangling divergence from the various updated patches. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
The Python3 find_package is more robust and allows more control over ABI and version selections.