Skip to content

Commit

Permalink
BUG: Avoid itkConfig double import trap
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hjmjohnson committed Nov 24, 2020
1 parent b011102 commit 1e334c9
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 17 deletions.
14 changes: 7 additions & 7 deletions Wrapping/Generators/Python/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions Wrapping/Generators/Python/WrapITK.pth.in
Original file line number Diff line number Diff line change
@@ -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@
11 changes: 8 additions & 3 deletions Wrapping/Generators/Python/itk/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 *
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Wrapping/Generators/Python/itk/support/itkBase.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
2 changes: 1 addition & 1 deletion Wrapping/Generators/Python/itk/support/itkExtras.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
4 changes: 2 additions & 2 deletions Wrapping/Generators/Python/itk/support/itkInitHelpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Wrapping/Generators/Python/itk/support/itkTemplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
File renamed without changes.

0 comments on commit 1e334c9

Please sign in to comment.