Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Force modern python3 find package and install of .pyi files #3560

Conversation

hjmjohnson
Copy link
Member

This PR represents the rabbit hole untangling of finding Python with a consistent version across internal remote module builds with python wrapping turned on.

The end goal was to test installing of .pyi files in the itk-stub directory for the purpose of creating wheels that provide type hinting to the IDE.

PR Checklist

@hjmjohnson hjmjohnson linked an issue Aug 19, 2022 that may be closed by this pull request
@hjmjohnson hjmjohnson self-assigned this Aug 19, 2022
@github-actions github-actions bot added area:Bridge Issues affecting the Bridge module type:Documentation Documentation improvement or change area:Python wrapping Python bindings for a class area:ThirdParty Issues affecting the ThirdParty module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots labels Aug 19, 2022
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some in-line comments.

CMake/WrappingConfigCommon.cmake Outdated Show resolved Hide resolved
Utilities/Maintenance/FindPython3_ModernizeCMake.sh Outdated Show resolved Hide resolved
@hjmjohnson hjmjohnson force-pushed the force-modern-python3-find_package branch from a8b0df7 to 335a6b0 Compare August 20, 2022 12:30
@hjmjohnson hjmjohnson changed the title WIP: Force modern python3 find package ENH: Force modern python3 find package and install of .pyi files Aug 20, 2022
@hjmjohnson hjmjohnson added this to the ITK 5.3.0 milestone Aug 20, 2022
hjmjohnson and others added 10 commits August 20, 2022 13:04
"\ingroup ITKMesh" not set in class TriangleMeshCurvatureCalculatorEnum.
ITK/Modules/Filtering/ImageGradient/include/itkGradientRecursiveGaussianImageFilter.hxx: In member function ‘GenerateData’:
ITK/Modules/Core/Common/include/itkDefaultPixelAccessor.h:72:5: warning: ‘MEM[(const struct CovariantVector &)&correctedGradient + 12]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   72 |     output = input;
      |     ^
ITK/Modules/Filtering/ImageGradient/include/itkGradientRecursiveGaussianImageFilter.h:214:29: note: ‘MEM[(const struct CovariantVector &)&correctedGradient + 12]’ was declared here
  214 |     OutputPixelType         correctedGradient;
      |                             ^
This duplicated block of code is now isolated into a separate included file
so that it may be more easily modified.

It is not clear if this code must be evaluated in both places
Now that cmake FindPython3 can be universally
used, prefer to only use that set of variable
names for identifying the python executables.

This maintenance script can be used to assist with moving from
the deprecated find_package(PythonInterp) and find_package(PythonLib)
to the preferred find_package(Python3) variable names.
Now that cmake FindPython3 can be universally
used, prefer to only use that set of variable
names for identifying the python executables.

Intial procesing with manual cleanup by:

ITK/Utilities/Maintenance/FindPython3_ModernizeCMake.sh
Incomplete refactoring seems to have been performed to
change the variable name.
Move the exposure of Python3_ROOT_DIR to a more central
place to where its value is set.
The remote modules need ITK_WRAP_PYTHON_VERSION defined in the ITKConfig.cmake and
also for embedded ITK builds.  The variable must be set before the
remote modules are processed.
When searching for python installation, ensure that a supported
version is found.
ITK needs to also install the stub files for providing
type hints to IDE's.
@hjmjohnson hjmjohnson force-pushed the force-modern-python3-find_package branch from 335a6b0 to cd5a604 Compare August 20, 2022 18:38
@github-actions github-actions bot added area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module type:Enhancement Improvement of existing methods or implementation labels Aug 20, 2022
@hjmjohnson
Copy link
Member Author

@dzenanz This is ready to review and rebase merge if the ghostflow-check-master can be overlooked until it is fixed.

This PR includes #3561 and #3562 in order to get this to be a clean build.

@thewtex This cleans up the python building environment, and provides an initial attempt at installing .pyi files for #3558, but I am not convinced that the installation is to the correct location for the wheel builds.

@dzenanz dzenanz merged commit 0af0a24 into InsightSoftwareConsortium:master Aug 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Bridge Issues affecting the Bridge module area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:Python wrapping Python bindings for a class area:ThirdParty Issues affecting the ThirdParty module type:Documentation Documentation improvement or change type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots
Projects
None yet
Development

Successfully merging this pull request may close these issues.

*.pyi files not installed
2 participants