Skip to content

Commit

Permalink
ENH: Rewrite python generation files with comments
Browse files Browse the repository at this point in the history
During the process of debugging the python wrapping code,
several code style conventions were implemented.  Primarily
in CMake code, set variables were 'unset' as soon as they
were no longer needed in order to limit their scope.

In python,the monolithic script was made more module, updated
to 'black' formatting conventions, added some typehints, and
corrected some python code improvement recommendations in order
to improve readability and debugging capabilities.  Additionally,
variables were made to have more local scope, and a limited number
of typehints were added to assist with tracing incorrect behavior.

- Remove non python wrapping optional code.
- Fix python coding recommendations
- Remove shadowed variable names, fix undefined escape sequence,
  use snake_case over CamelCase.
- Fix igenerator.py coding recommendations
- Remove shadowed variable names, fix undefined escape sequence,
  use snake_case over CamelCase.
- index.txt files belong in pkl_dir
- Move setting ITK_STUB_DIR/ITK_PKL_DIR to correct scope.
- Conform to python coding style
- Changed camel case to snake case.  Removed
  shadowed variables. Make formatting consistent.
- Added typehints, and simplified trivial code tests.
- These changes will make future changes easier
  to identify.
- Add new input to explicitly allow monitoring and tracking of
  required biproduct files.
- Updated debugging ITK_PYI_INDEX_FILE_SETTING.
- Remove SWIG_INTERFACE_MODULE ambiguity.
- Minimize scope of of variables in cmake.
- Make list of proxy and template imports, and add debugging capabilities
- More prepend/append changes.
- Decouple CASTXML_INCLUDE, SWIG_INCLUDE from WRAPPER_INCLUDE_FILES
- Remove unnecessary unsets
- test that WRAPPER_LIBRARY_PYTHON is required in all cases.
- Remove checking mandatory WRAPPER_LIBRARY_PYTHON
- Prefer string APPEND/PREPEND for ITK_WRAP_PYTHON_SWIG_EXT
- Put temporary generated wrapping files in named subdirectories.
- This greatly cleans up the directory tree so that it is easier
  to trace which process generates which files.
- Encapsulate test_lib_module_names_different
- Remove outdated comments.
- The logic for when to add subdirectories for
  PyUtils and PyBase was unnecessarily complicated now
  that only Python wrapping is supported.
- Adding debugging code for wrapping
- The wrapping code contaminates the global namespace.
  These debugging functions are useful for identifying
  changed values in macros.
- Restructured python type hint generation procedure.  A few
  inter-submodule dependencies were causing a few classes typehints to
  fail.  This commit restructures the generation process to allow for
  these dependencies.  The generation occurs in two stages:
      1. pickle files are saved for each class defined for each submodule in igenerator
            (Some classes are defined multiple times (once for each submodule))
      2. Pickle files are merged and stub files are produced in pyi_generator
            (This runs once after all runs of igenerator have finished)
  Currently pyi_generator runs before igenerator, this is a problem.
  A dependency needs to be added to cause pyi_generator to be ran last.

Co-authored-by: Kian Weimer <kianweimer@gmail.com>
  • Loading branch information
hjmjohnson and kian-weimer committed May 11, 2022
1 parent 37239fe commit 9e5d62d
Show file tree
Hide file tree
Showing 12 changed files with 2,535 additions and 2,012 deletions.
1 change: 0 additions & 1 deletion CMake/ITKConfig.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ set(ITK_USE_GPU "@ITK_USE_GPU@")

# Wrapping
set(ITK_WRAPPING "@ITK_WRAPPING@")
# ITK_WRAP_SWIGINTERFACE and ITK_WRAP_CASTXML are set to ITK_WRAPPING value.
# ITK_WRAP_DOC is disabled by default.

if( NOT DEFINED ITK_WRAP_PYTHON)
Expand Down
4 changes: 2 additions & 2 deletions Modules/Core/Common/wrapping/itkImageDuplicator.wrap
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ macro(EXTEND_IMAGE_DUPLICATOR type)
set(import_text "%import pyCommon.i\n")
string(FIND ${ITK_WRAP_PYTHON_SWIG_EXT} ${import_text} pos)
if(${pos} EQUAL -1)
set(ITK_WRAP_PYTHON_SWIG_EXT "${import_text}${ITK_WRAP_PYTHON_SWIG_EXT}")
string(PREPEND ITK_WRAP_PYTHON_SWIG_EXT "${import_text}")
endif()
set(ITK_WRAP_PYTHON_SWIG_EXT "${ITK_WRAP_PYTHON_SWIG_EXT}DECL_PYTHON_FORCE_SNAKE_CASE_CLASS(itkImageDuplicator${type})\n")
string(APPEND ITK_WRAP_PYTHON_SWIG_EXT "DECL_PYTHON_FORCE_SNAKE_CASE_CLASS(itkImageDuplicator${type})\n")
endif()
endmacro()

Expand Down
64 changes: 45 additions & 19 deletions Wrapping/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ if(NOT WRAP_ITK_INSTALL_COMPONENT_IDENTIFIER)
set(WRAP_ITK_INSTALL_COMPONENT_IDENTIFIER "")
endif()

unset(GLOBAL_IdxFilesList)
unset(GLOBAL_IdxFilesList CACHE)

# Set WRAP_ITK_INSTALL_COMPONENT_PER_MODULE to 1 to have wrapping install
# component names prefixed with their respective module name.
# This can be used to have fine-control over the split of installation.
Expand All @@ -80,6 +83,10 @@ set(CXX_TEST_PATH ${EXECUTABLE_OUTPUT_PATH})
# Additional files for installation
###############################################################################

# ITK_STUB_DIR: all stub files are stored in this directory
set(ITK_STUB_DIR "${ITK_DIR}/Wrapping/Generators/Python/itk-stubs" CACHE INTERNAL "where python interface files are stored.")
# ITK_PKL_DIR: all temporary pickled object AST representations stored here
set(ITK_PKL_DIR "${ITK_DIR}/Wrapping/Generators/Python/itk-pkl" CACHE INTERNAL "where temp pkl files are stored")
set(WRAP_ITK_TYPEDEFS_DIRECTORY "${ITK_DIR}/Wrapping/Typedefs")
set(WRAP_ITK_LIB_DIRECTORY "${CMAKE_LIBRARY_OUTPUT_DIRECTORY}")

Expand All @@ -95,13 +102,6 @@ set(ITK_TEST_DRIVER "itkTestDriver")

include(ConfigureWrapping.cmake)

###############################################################################
# let the different generators running some code before begining to parse the
# modules
###############################################################################

itk_wrap_modules()

###############################################################################
# Configure specific wrapper modules
###############################################################################
Expand Down Expand Up @@ -136,22 +136,48 @@ set(WRAP_ITK_MODULES ${WRAP_ITK_MODULES} CACHE INTERNAL "Internal library list."
# modules
###############################################################################

itk_end_wrap_modules_all_generators()
if(${module_prefix}_WRAP_PYTHON)
# Wrap PyUtils
if(NOT EXTERNAL_WRAP_ITK_PROJECT)
add_subdirectory(${ITK_WRAP_PYTHON_SOURCE_DIR}/PyUtils)
add_subdirectory(${ITK_WRAP_PYTHON_SOURCE_DIR}/PyBase)
endif()
endif()


###############################################################################
# Generate Python typehints when wrapped
###############################################################################
if(ITK_WRAP_PYTHON)
# Generate __init__.pyi file with wrapped templates/interfaces
set(ITK_STUB_TEMPLATE_IMPORTS "$ENV{ITK_STUB_TEMPLATE_IMPORTS}")
list(SORT ITK_STUB_TEMPLATE_IMPORTS CASE INSENSITIVE)
list(JOIN ITK_STUB_TEMPLATE_IMPORTS "\n" ITK_STUB_TEMPLATE_IMPORTS)
configure_file(Generators/Python/itk/__init__.pyi.in Generators/Python/itk-stubs/__init__.pyi)

# Generate _proxies.pyi file with wrapped proxy classes and methods
set(ITK_STUB_PROXY_IMPORTS "$ENV{ITK_STUB_PROXY_IMPORTS}")
list(SORT ITK_STUB_PROXY_IMPORTS CASE INSENSITIVE)
list(JOIN ITK_STUB_PROXY_IMPORTS "\n" ITK_STUB_PROXY_IMPORTS)
configure_file(Generators/Python/itk/_proxies.pyi.in Generators/Python/itk-stubs/_proxies.pyi)
# Generate .pyi files for each class
# The intent is that the pyi_generator file is run after every occurrence of igenerator has ran.
# The call to igenerator is contained within a macro that is called by each of the modules.
# According to online documents CMAkE dependencies can only be created between
# custom_targets and custom_commands contained within the same CMakeLists file. It is unknown if this
# can be done since the macros are called from a variety of locations.

# PYI_GENERATOR: The file location of the pyi_generator.py script
# This script is responsible for merging all of the pickle files together and
# generating the Template and Proxy files for each class. In addition this file
# generates the __init__.pyi and _proxies.pyi files used to interface between
# the individual .pyi files.
set(PYI_GENERATOR "${CMAKE_CURRENT_SOURCE_DIR}/Generators/Python/itk/pyi_generator.py")

# Run pyi_generator
# ${GLOBAL_IdxFilesList}: is described within SwigInterface/CMakeLists.txt
# The variable is passed to pyi_generator to make sure that the function only uses current index files
# All index files found that are not in the given list are assumed to be outdated and are removed.
add_custom_target(itk-stub-files ALL
BYPRODUCTS "${ITK_STUB_DIR}/_proxies.pyi" "${ITK_STUB_DIR}/__init__.pyi"
COMMAND ${Python3_EXECUTABLE} ${PYI_GENERATOR}
--pyi_dir "${ITK_STUB_DIR}"
--pkl_dir "${ITK_PKL_DIR}"
--index_files "${GLOBAL_IdxFilesList}"
DEPENDS ${PYI_GENERATOR} ${GLOBAL_IdxFilesList}
COMMENT "Generating .pyi files for Python wrapper interface"
VERBATIM
)

unset(ITK_STUB_DIR CACHE)
unset(ITK_PKL_DIR CACHE)
endif()
17 changes: 11 additions & 6 deletions Wrapping/Generators/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,25 @@ endif()



if(DEFINED ITK_WRAP_GCCXML)
message(FATAL_ERROR "ITK_WRAP_GCCXML is deprecated. Use ${module_prefix}_WRAP_CASTXML instead.")
endif()
###############################################################################
# build the generators list
if(EXTERNAL_WRAP_ITK_PROJECT)
# generators there have been turned on while building wrapitk
else()
if(DEFINED ITK_WRAP_GCCXML)
# Keep ITK_WRAP_GCCXML for backward compatibility.
set(${module_prefix}_WRAP_CASTXML "${ITK_WRAP_GCCXML}" CACHE STRING "Build xml files.")
else()
set(${module_prefix}_WRAP_CASTXML ON CACHE BOOL "Build xml files.")
endif()
cmake_dependent_option(${module_prefix}_WRAP_CASTXML "Build xml files." ON ITK_WRAP_PYTHON OFF)
cmake_dependent_option(${module_prefix}_WRAP_SWIGINTERFACE "Build swig interfaces." ON ITK_WRAP_PYTHON OFF)
mark_as_advanced(${module_prefix}_WRAP_CASTXML ${module_prefix}_WRAP_SWIGINTERFACE)
endif()
if(NOT ${module_prefix}_WRAP_CASTXML)
message(FATAL_ERROR "${module_prefix}_WRAP_CASTXML is required to be on for wrapping python")
endif()
if(NOT ${module_prefix}_WRAP_SWIGINTERFACE)
message(FATAL_ERROR "${module_prefix}_WRAP_SWIGINTERFACE is required to be on for wrapping python")
endif()


set(WRAP_ITK_GENERATORS CACHE INTERNAL "Internal generators list.")
set(WRAP_ITK_UNUSED_GENERATORS CACHE INTERNAL "Internal unused generators list.")
Expand Down
16 changes: 8 additions & 8 deletions Wrapping/Generators/Python/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ macro(itk_end_wrap_submodule_python group_name)
ADD_PYTHON_CONFIG_TEMPLATE("map" "std::map" "mapULitkCellInterfaceDQEMCTI${d}" "unsigned long, itk::CellInterface< double, itk::QuadEdgeMeshCellTraitsInfo< ${d} > >*")
endforeach()

set(ITK_WRAP_PYTHON_SWIG_EXT "${ITK_WRAP_PYTHON_SWIG_EXT}${text}")
string(APPEND ITK_WRAP_PYTHON_SWIG_EXT "${text}")
unset(text)
endif()

Expand All @@ -310,14 +310,14 @@ macro(itk_end_wrap_submodule_python group_name)
if(${module_prefix}_WRAP_DOC AND NOT ITK_WRAP_PYTHON_PROCESS_SWIG_INPUTS)
# yes. Include the docstring file
set(doc_file "${WRAPPER_MASTER_INDEX_OUTPUT_DIR}/${group_name}_doc.i")
set(ITK_WRAP_PYTHON_SWIG_EXT "%include ${group_name}_doc.i\n\n${ITK_WRAP_PYTHON_SWIG_EXT}")
string(PREPEND ITK_WRAP_PYTHON_SWIG_EXT "%include ${group_name}_doc.i\n\n")
else()
# no. Clear the doc_file var
set(doc_file "")
endif()

# the default typemaps, exception handler, and includes
set(ITK_WRAP_PYTHON_SWIG_EXT "%import pyBase.i\n\n${ITK_WRAP_PYTHON_SWIG_EXT}")
string(PREPEND ITK_WRAP_PYTHON_SWIG_EXT "%import pyBase.i\n\n")

# create the swig interface for all the groups in the module
set(interface_file "${WRAPPER_MASTER_INDEX_OUTPUT_DIR}/${base_name}.i")
Expand Down Expand Up @@ -392,12 +392,12 @@ endmacro()


macro(ADD_PYTHON_OUTPUT_RETURN_BY_VALUE_CLASS type function)
set(ITK_WRAP_PYTHON_SWIG_EXT "${ITK_WRAP_PYTHON_SWIG_EXT}DECL_PYTHON_OUTPUT_RETURN_BY_VALUE_CLASS(${type}, ${function})\n")
string(APPEND ITK_WRAP_PYTHON_SWIG_EXT "DECL_PYTHON_OUTPUT_RETURN_BY_VALUE_CLASS(${type}, ${function})\n")
endmacro()


macro(ADD_PYTHON_SEQ_TYPEMAP swig_name dim)
set(ITK_WRAP_PYTHON_SWIG_EXT "${ITK_WRAP_PYTHON_SWIG_EXT}DECL_PYTHON_SEQ_TYPEMAP(${swig_name}, ${dim})\n")
string(APPEND ITK_WRAP_PYTHON_SWIG_EXT "DECL_PYTHON_SEQ_TYPEMAP(${swig_name}, ${dim})\n")
endmacro()


Expand All @@ -408,19 +408,19 @@ macro(ADD_PYTHON_VEC_TYPEMAP swig_name template_params)
# in C/C++ macros
string(REGEX MATCH "(.*,.*)" isTemplate "${type}")
if(NOT isTemplate)
set(ITK_WRAP_PYTHON_SWIG_EXT "${ITK_WRAP_PYTHON_SWIG_EXT}DECL_PYTHON_VEC_TYPEMAP(${swig_name}, ${type}, ${dim})\n")
string(APPEND ITK_WRAP_PYTHON_SWIG_EXT "DECL_PYTHON_VEC_TYPEMAP(${swig_name}, ${type}, ${dim})\n")
endif()
endmacro()

macro(ADD_PYTHON_VARIABLE_LENGTH_SEQ_TYPEMAP type value_type)
set(ITK_WRAP_PYTHON_SWIG_EXT "${ITK_WRAP_PYTHON_SWIG_EXT}DECL_PYTHON_VARLEN_SEQ_TYPEMAP(${type}, ${value_type})\n")
string(APPEND ITK_WRAP_PYTHON_SWIG_EXT "DECL_PYTHON_VARLEN_SEQ_TYPEMAP(${type}, ${value_type})\n")
endmacro()


macro(ADD_PYTHON_POINTER_TYPEMAP typemap_name)
set(text "DECLARE_REF_COUNT_CLASS(${typemap_name})\n")

set(ITK_WRAP_PYTHON_SWIG_EXT "${text}${ITK_WRAP_PYTHON_SWIG_EXT}")
string(PREPEND ITK_WRAP_PYTHON_SWIG_EXT "${text}")
endmacro()

###############################################################################
Expand Down
Loading

0 comments on commit 9e5d62d

Please sign in to comment.