From 1e334c9aab3d6eb840ea80731d6b34b01ab25f99 Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Tue, 24 Nov 2020 10:08:54 -0600 Subject: [PATCH] BUG: Avoid itkConfig double import trap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This potential failure mode can be summed up in the following general guideline: “Never add a package directory, or any directory inside a package, directly to the Python path”. The reason that adding nested directories to the Python sys.path is problematic is that every module in that directory is now potentially accessible under two different names: as a top level module (since the directory is on sys.path) and as a submodule of the package (if the higher level directory containing the package itself is also on sys.path). We must import the itkConfig module from python path, not local reference in order to maintain the desired behavior that the module acts as a singleton for the module values (i.e. once the itkConfig module is loaded, we assume it is never loaded again, thus changed values like itkConfig.LazyLoading persist across diffent modules). `import .conf.itkConfig` is a different context than `import itkConfig`, even if they are the same file. The LazyLoading and other values may be different in the two contexts. In order to maintain backward compatibility AND avoid the double import trap by “Never add a package directory, or any directory inside a package, directly to the Python path”, the itkConfig.py file must exist outside of the itk package directory (it *must* be a stand-alone single file module). http://python-notes.curiousefficiency.org/en/latest/python_concepts/import_traps.html --- Wrapping/Generators/Python/CMakeLists.txt | 14 +++++++------- Wrapping/Generators/Python/WrapITK.pth.in | 4 ++-- Wrapping/Generators/Python/itk/__init__.py | 11 ++++++++--- Wrapping/Generators/Python/itk/support/itkBase.py | 2 +- .../Generators/Python/itk/support/itkExtras.py | 2 +- .../Python/itk/support/itkInitHelpers.py | 4 ++-- .../Generators/Python/itk/support/itkTemplate.py | 2 +- .../Python/{itk/conf => }/itkConfig.py.in | 0 8 files changed, 22 insertions(+), 17 deletions(-) rename Wrapping/Generators/Python/{itk/conf => }/itkConfig.py.in (100%) diff --git a/Wrapping/Generators/Python/CMakeLists.txt b/Wrapping/Generators/Python/CMakeLists.txt index e50e3053ae7..525788720a5 100644 --- a/Wrapping/Generators/Python/CMakeLists.txt +++ b/Wrapping/Generators/Python/CMakeLists.txt @@ -14,7 +14,7 @@ if(NOT EXTERNAL_WRAP_ITK_PROJECT) else() set(ITK_WRAP_PYTHON_ROOT_BINARY_DIR "${WRAPPER_LIBRARY_OUTPUT_DIR}/Generators/Python" CACHE INTERNAL "python binary dir") endif() -set(ITK_PYTHON_PACKAGE_CONFIG_SUPPORT_DIR "${ITK_WRAP_PYTHON_ROOT_BINARY_DIR}/itk/conf" CACHE INTERNAL "python binary dir") +set(ITK_PYTHON_PACKAGE_CONFIG_SUPPORT_DIR "${ITK_WRAP_PYTHON_ROOT_BINARY_DIR}" CACHE INTERNAL "python binary dir") set(ITK_WRAP_PYTHON_SWIG_CONFIGURATION_DIR "${ITK_WRAP_PYTHON_ROOT_BINARY_DIR}/itk/Configuration" CACHE INTERNAL "python binary dir") ############################################################################### # create the python directory in the classindex dir @@ -76,7 +76,7 @@ if(NOT EXTERNAL_WRAP_ITK_PROJECT) set(ITK_PYTHON_SWIG_LIBS_DIR "${ITK_WRAP_PYTHON_ROOT_BINARY_DIR}/itk" CACHE INTERNAL "where the swig shared libs are place _ITK*.so files") file(RELATIVE_PATH CONFIG_PYTHON_SWIGPY_DIR ${ITK_PYTHON_PACKAGE_CONFIG_SUPPORT_DIR} ${ITK_PYTHON_SWIG_LIBS_DIR}) file(RELATIVE_PATH CONFIG_PYTHON_SWIGLIB_DIR ${ITK_PYTHON_PACKAGE_CONFIG_SUPPORT_DIR} ${ITK_PYTHON_SWIG_LIBS_DIR}) - configure_file("${CMAKE_CURRENT_SOURCE_DIR}/itk/conf/itkConfig.py.in" + configure_file("${CMAKE_CURRENT_SOURCE_DIR}/itkConfig.py.in" "${ITK_WRAP_PYTHON_ROOT_BINARY_DIR}/InstallOnly/itkConfig.py" @ONLY) @@ -99,7 +99,7 @@ if(NOT EXTERNAL_WRAP_ITK_PROJECT) file(RELATIVE_PATH CONFIG_PYTHON_SWIGPY_DIR ${ITK_PYTHON_PACKAGE_CONFIG_SUPPORT_DIR} ${ITK_PYTHON_SWIG_LIBS_DIR}) file(RELATIVE_PATH CONFIG_PYTHON_SWIGLIB_DIR ${ITK_PYTHON_PACKAGE_CONFIG_SUPPORT_DIR} ${ITK_PYTHON_SWIG_LIBS_DIR}) - configure_file("${CMAKE_CURRENT_SOURCE_DIR}/itk/conf/itkConfig.py.in" + configure_file("${CMAKE_CURRENT_SOURCE_DIR}/itkConfig.py.in" "${ITK_PYTHON_PACKAGE_CONFIG_SUPPORT_DIR}/${config}/itkConfig.py" @ONLY) endforeach() @@ -199,16 +199,16 @@ endif() if(CMAKE_CONFIGURATION_TYPES) foreach(config ${CMAKE_CONFIGURATION_TYPES}) - set(CONFIG_ITK_WRAP_PYTHON_DIR "${CMAKE_CURRENT_BINARY_DIR}/itk/conf/${config}") - set(CONFIG_ITK_WRAP_LIBRARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/${config}/itk") + set(CONFIG_ITK_WRAP_PYTHON_DIR "${ITK_PYTHON_PACKAGE_CONFIG_SUPPORT_DIR}/${config}") + #set(CONFIG_ITK_WRAP_LIBRARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/${config}/itk") configure_file("${CMAKE_CURRENT_SOURCE_DIR}/WrapITK.pth.in" "${CMAKE_CURRENT_BINARY_DIR}/${config}/WrapITK.pth" @ONLY) endforeach() else() - set(CONFIG_ITK_WRAP_PYTHON_DIR "${CMAKE_CURRENT_BINARY_DIR}/itk/conf") - set(CONFIG_ITK_WRAP_LIBRARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/itk") + set(CONFIG_ITK_WRAP_PYTHON_DIR "${ITK_PYTHON_PACKAGE_CONFIG_SUPPORT_DIR}") + #set(CONFIG_ITK_WRAP_LIBRARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/itk") configure_file("${CMAKE_CURRENT_SOURCE_DIR}/WrapITK.pth.in" "${CMAKE_CURRENT_BINARY_DIR}/WrapITK.pth" @ONLY) diff --git a/Wrapping/Generators/Python/WrapITK.pth.in b/Wrapping/Generators/Python/WrapITK.pth.in index 7e2dc5a4b96..163ffd8a3c9 100644 --- a/Wrapping/Generators/Python/WrapITK.pth.in +++ b/Wrapping/Generators/Python/WrapITK.pth.in @@ -1,3 +1,3 @@ -# Python pth file to add WrapITK's path to sys.path. +# Python pth file to add itk package path to sys.path +# for itk version "@ITK_VERSION_MAJOR@.@ITK_VERSION_MINOR@.@ITK_VERSION_PATCH@" @CONFIG_ITK_WRAP_PYTHON_DIR@ -@CONFIG_ITK_WRAP_LIBRARY_DIR@ diff --git a/Wrapping/Generators/Python/itk/__init__.py b/Wrapping/Generators/Python/itk/__init__.py index fca972f523a..1321d35c397 100644 --- a/Wrapping/Generators/Python/itk/__init__.py +++ b/Wrapping/Generators/Python/itk/__init__.py @@ -18,7 +18,12 @@ """itk: Top-level container package for ITK wrappers.""" -from .conf.itkConfig import ITK_GLOBAL_VERSION_STRING as __version__ +# Must import module from python path, not local reference +# in order to maintain the singular value of the module values. +# `import .conf.itkConfig` is a different context than +# `import itkConfig`, even if they are the same file. The +# LazyLoading and other values may be different in the two contexts. +from itkConfig import ITK_GLOBAL_VERSION_STRING as __version__ from .support.itkExtras import * from .support.itkInitHelpers import * @@ -48,10 +53,10 @@ def _get_lazy_attributes(local_lazy_attributes, l_module, l_data): from .support import itkBase as _itkBase from .support import itkLazy as _itkLazy - from .conf import itkConfig as _itkConfig + from itkConfig import LazyLoading as _LazyLoading import sys - if _itkConfig.LazyLoading: + if _LazyLoading: # If we are loading lazily (on-demand), make a dict mapping the available # classes/functions/etc. (read from the configuration modules) to the # modules they are declared in. Then pass that dict to a LazyITKModule diff --git a/Wrapping/Generators/Python/itk/support/itkBase.py b/Wrapping/Generators/Python/itk/support/itkBase.py index 82693e34db0..bb8a0bf7fd5 100644 --- a/Wrapping/Generators/Python/itk/support/itkBase.py +++ b/Wrapping/Generators/Python/itk/support/itkBase.py @@ -23,7 +23,7 @@ # Required to work around weird import error with xarray import pkg_resources -from itk.conf import itkConfig +import itkConfig from itk.support.itkTemplate import itkTemplate diff --git a/Wrapping/Generators/Python/itk/support/itkExtras.py b/Wrapping/Generators/Python/itk/support/itkExtras.py index e2e0aead3f1..ce70779b2ef 100644 --- a/Wrapping/Generators/Python/itk/support/itkExtras.py +++ b/Wrapping/Generators/Python/itk/support/itkExtras.py @@ -1573,7 +1573,7 @@ def tuple_to_string_type(t): # install progress callback and custom completer if we are in ipython # interpreter try: - from itk.conf import itkConfig + import itkConfig import IPython if IPython.get_ipython(): diff --git a/Wrapping/Generators/Python/itk/support/itkInitHelpers.py b/Wrapping/Generators/Python/itk/support/itkInitHelpers.py index 476eeb348cf..3bbb380e6db 100644 --- a/Wrapping/Generators/Python/itk/support/itkInitHelpers.py +++ b/Wrapping/Generators/Python/itk/support/itkInitHelpers.py @@ -12,7 +12,7 @@ def auto_not_in_place(v=True): """Force it to not run in place""" - from itk.conf import itkConfig + import itkConfig itkConfig.NotInPlace = v @@ -25,7 +25,7 @@ def auto_progress(progress_type=1): 2 -> simple auto progress (without special characters) 0 or False -> disable auto progress """ - from itk.conf import itkConfig + import itkConfig if progress_type is True or progress_type == 1: itkConfig.ImportCallback = terminal_import_callback diff --git a/Wrapping/Generators/Python/itk/support/itkTemplate.py b/Wrapping/Generators/Python/itk/support/itkTemplate.py index b13afa396e6..535d6ab095e 100644 --- a/Wrapping/Generators/Python/itk/support/itkTemplate.py +++ b/Wrapping/Generators/Python/itk/support/itkTemplate.py @@ -25,7 +25,7 @@ import warnings from typing import Dict, Any -from ..conf import itkConfig +import itkConfig from ..support import itkBase from ..support.itkExtras import output diff --git a/Wrapping/Generators/Python/itk/conf/itkConfig.py.in b/Wrapping/Generators/Python/itkConfig.py.in similarity index 100% rename from Wrapping/Generators/Python/itk/conf/itkConfig.py.in rename to Wrapping/Generators/Python/itkConfig.py.in