diff --git a/autowrap/CodeGenerator.py b/autowrap/CodeGenerator.py index 9828a25..5dba44c 100644 --- a/autowrap/CodeGenerator.py +++ b/autowrap/CodeGenerator.py @@ -279,6 +279,7 @@ def create_pyx_file(self, debug: bool = False) -> None: self.create_cimports() self.create_foreign_cimports() self.create_foreign_enum_imports() + self.create_scoped_enum_helpers() self.create_includes() def create_for( @@ -521,6 +522,10 @@ def create_wrapper_for_enum( """ ) + # Register scoped enums in the module registry for cross-module lookup + if decl.scoped: + code.add("_scoped_enum_registry['$name'] = $name", name=name) + # TODO check if we need to add an import or custom type to type stubs for enums out_codes[decl.name] = code out_stub_codes[decl.name] = stub_code @@ -2088,37 +2093,87 @@ def create_foreign_enum_imports(self): Background: In multi-module builds (e.g., pyOpenMS splits wrappers across _pyopenms_1.pyx through _pyopenms_8.pyx), scoped enums (enum class) are wrapped as Python IntEnum classes (e.g., _PySpectrumType). When module A - uses an enum defined in module B in a type assertion like: - - assert isinstance(in_0, _PySpectrumType) - - the Python class _PySpectrumType must be available. + uses an enum defined in module B, the Python class must be accessible. Problem: Adding module-level imports like: from ._pyopenms_3 import _PySpectrumType - causes circular import errors. The modules form an import chain during - initialization (1 -> 8 -> 7 -> ... -> 2), and when module 2 tries to - import from module 3, module 3 hasn't finished initializing yet. + causes circular import errors during module initialization. - Solution: Instead of module-level imports, we use globals().get() for - late binding in type assertions: - - assert isinstance(in_0, globals().get('_PySpectrumType', int)) + Solution: Each module has: + 1. A local registry (_scoped_enum_registry) that stores enums defined in that module + 2. A lookup function (_get_scoped_enum_class) that: + - First checks the local registry (fast path for same-module enums) + - Then searches all loaded pyopenms modules via sys.modules + - Caches results for subsequent lookups This approach: - - Compiles successfully (globals().get() is always valid Python) - - Resolves the enum class at runtime after all modules are loaded - - Falls back to 'int' which works for IntEnum (inherits from int) + - Avoids circular imports (no module-level cross-imports) + - Provides fast lookup for same-module enums (local dict lookup) + - Works across modules by searching sys.modules at runtime + - Maintains proper type checking and enum wrapping - See EnumConverter.type_check_expression() in ConversionProvider.py for - the implementation. + See EnumConverter in ConversionProvider.py and create_default_cimports() + for the implementation details. """ - # No-op: cross-module imports at module load time cause circular imports - # The enum classes are resolved at runtime via globals().get() + # No-op: cross-module enum lookup is handled by _get_scoped_enum_class() pass + def create_scoped_enum_helpers(self): + """Generate helper functions for scoped enum lookup. + + This adds a registry and lookup function to the pyx file (not pxd) that + enables cross-module scoped enum resolution in multi-module builds. + + The generated code includes: + 1. _scoped_enum_registry: A dict mapping enum names to their Python classes + 2. _get_scoped_enum_class(): A function that looks up enum classes by name, + first checking the local registry, then searching all loaded modules. + + This is added to top_level_pyx_code so it only appears in .pyx files, + not .pxd files (which cannot contain def statements). + """ + code = Code() + code.add( + """ + |# Registry for scoped enum classes defined in this module + |_scoped_enum_registry = {} + | + |def _get_scoped_enum_class(name, fallback=None): + | '''Look up a scoped enum class by name, searching across all pyopenms modules. + | + | In multi-module builds (e.g., pyOpenMS), scoped enums may be defined in one + | module but used in another. This function provides cross-module lookup by: + | 1. First checking the local module registry (fastest path) + | 2. Then searching all loaded pyopenms modules via sys.modules + | + | Args: + | name: The enum class name (e.g., '_PySpectrumType') + | fallback: Value to return if enum not found. Defaults to int for type checks, + | or can be set to a callable like (lambda x: x) for conversions. + | + | Returns the enum class if found, or the fallback value if not found. + | ''' + | if fallback is None: + | fallback = int + | # Fast path: check local registry first + | if name in _scoped_enum_registry: + | return _scoped_enum_registry[name] + | # Slow path: search all loaded pyopenms modules + | for mod_name, mod in list(_sys.modules.items()): + | if mod is not None and (mod_name.startswith('pyopenms.') or mod_name == 'pyopenms'): + | enum_cls = getattr(mod, name, None) + | if enum_cls is not None: + | # Cache for future lookups + | _scoped_enum_registry[name] = enum_cls + | return enum_cls + | # Fallback + | return fallback + """ + ) + self.top_level_pyx_code.append(code) + def create_cimports(self): self.create_std_cimports() code = Code() @@ -2151,6 +2206,7 @@ def create_default_cimports(self): |#cython: c_string_encoding=ascii |#cython: embedsignature=False |from enum import IntEnum as _PyEnum + |import sys as _sys |from cpython cimport Py_buffer |from cpython cimport bool as pybool_t |from libcpp.string cimport string as libcpp_string diff --git a/autowrap/ConversionProvider.py b/autowrap/ConversionProvider.py index f201c35..3d35115 100644 --- a/autowrap/ConversionProvider.py +++ b/autowrap/ConversionProvider.py @@ -419,20 +419,16 @@ def type_check_expression(self, cpp_type: CppType, argument_var: str) -> str: # # Problem: Scoped enums (enum class) are wrapped as Python IntEnum classes # (e.g., _PySpectrumType). When module A uses an enum defined in module B, - # the isinstance check needs access to that Python class. Two issues arise: + # the isinstance check needs access to that Python class. # - # 1. Compile-time: Cython needs the name to be declared/imported - # 2. Runtime: Module-level imports cause circular import errors because - # all modules import from each other during initialization - # - # Solution: Use globals().get() for late binding. This: - # - Allows compilation (globals().get() is always valid Python syntax) - # - Resolves the enum class at runtime after all modules are loaded + # Solution: Use _get_scoped_enum_class() for cross-module lookup. This: + # - First checks the local module's registry (fast path for same-module enums) + # - Then searches all loaded pyopenms modules via sys.modules # - Falls back to 'int' which works for IntEnum values (IntEnum inherits from int) # - # See also: CodeGenerator.create_foreign_enum_imports() which documents - # why module-level imports are avoided. - return "isinstance(%s, globals().get('%s', int))" % (argument_var, name) + # See also: CodeGenerator.create_default_cimports() where the lookup function + # and registry are defined. + return "isinstance(%s, _get_scoped_enum_class('%s'))" % (argument_var, name) def input_conversion( self, cpp_type: CppType, argument_var: str, arg_num: int @@ -453,13 +449,13 @@ def output_conversion( return "%s = %s" % (output_py_var, input_cpp_var) else: # For scoped enums, wrap the int value in the Python enum class - # Use globals().get() for late binding to avoid circular import issues + # Use _get_scoped_enum_class() for cross-module lookup # in multi-module builds (see type_check_expression for details) if self.enum.cpp_decl.annotations.get("wrap-attach"): name = "_Py" + self.enum.name else: name = self.enum.name - return "%s = globals().get('%s', lambda x: x)(%s)" % (output_py_var, name, input_cpp_var) + return "%s = _get_scoped_enum_class('%s', lambda x: x)(%s)" % (output_py_var, name, input_cpp_var) class CharConverter(TypeConverterBase): diff --git a/autowrap/version.py b/autowrap/version.py index 4de4940..4300798 100644 --- a/autowrap/version.py +++ b/autowrap/version.py @@ -30,10 +30,10 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. """ -__version__ = "0.26.1" +__version__ = "0.26.2" # For backward compatibility with older code expecting tuple format -__version_tuple__ = (0, 26, 1) +__version_tuple__ = (0, 26, 2) # for compatibility with older version: version = __version_tuple__ diff --git a/tests/test_code_generator.py b/tests/test_code_generator.py index 8bd5d13..bfcf4bd 100644 --- a/tests/test_code_generator.py +++ b/tests/test_code_generator.py @@ -861,13 +861,14 @@ def test_cross_module_scoped_enum_imports(tmpdir): f"EnumConsumer.pyx content:\n{consumer_pyx}" ) - # Test 2: Verify isinstance checks use globals().get() for late binding - assert "globals().get('_PyTask_TaskStatus'" in consumer_pyx, ( - f"Expected isinstance check with globals().get('_PyTask_TaskStatus', int) for wrap-attach enum.\n" + # Test 2: Verify isinstance checks use _get_scoped_enum_class() helper for late binding + # The helper looks up enums via registry and sys.modules for cross-module support + assert "_get_scoped_enum_class('_PyTask_TaskStatus')" in consumer_pyx, ( + f"Expected isinstance check with _get_scoped_enum_class('_PyTask_TaskStatus') for wrap-attach enum.\n" f"EnumConsumer.pyx content:\n{consumer_pyx}" ) - assert "globals().get('Priority'" in consumer_pyx, ( - f"Expected isinstance check with globals().get('Priority', int) for non-wrap-attach enum.\n" + assert "_get_scoped_enum_class('Priority')" in consumer_pyx, ( + f"Expected isinstance check with _get_scoped_enum_class('Priority') for non-wrap-attach enum.\n" f"EnumConsumer.pyx content:\n{consumer_pyx}" ) @@ -976,6 +977,27 @@ def test_cross_module_scoped_enum_imports(tmpdir): except AssertionError as e: assert "wrong type" in str(e) + # Runtime Test 8: Cross-module getter→setter roundtrip (the pyOpenMS scenario) + # This tests that a class in module B can use getX()/setX() with an enum from module A + # where setX(getX()) works correctly - this was the missing test case. + tracker = EnumConsumer.StatusTracker() + + # Test with class-attached enum (Task::TaskStatus) + assert tracker.getStatus() == EnumProvider.Task.TaskStatus.PENDING + tracker.setStatus(EnumProvider.Task.TaskStatus.RUNNING) + assert tracker.getStatus() == EnumProvider.Task.TaskStatus.RUNNING + # THE KEY TEST: getter→setter roundtrip across module boundaries + tracker.setStatus(tracker.getStatus()) # This is what failed in pyOpenMS! + assert tracker.getStatus() == EnumProvider.Task.TaskStatus.RUNNING + + # Test with standalone enum (Priority) + assert tracker.getPriority() == EnumProvider.Priority.LOW + tracker.setPriority(EnumProvider.Priority.HIGH) + assert tracker.getPriority() == EnumProvider.Priority.HIGH + # THE KEY TEST: getter→setter roundtrip across module boundaries + tracker.setPriority(tracker.getPriority()) # This is what failed in pyOpenMS! + assert tracker.getPriority() == EnumProvider.Priority.HIGH + print("Test passed: Cross-module scoped enum imports work correctly at runtime!") finally: diff --git a/tests/test_files/enum_cross_module/EnumConsumer.hpp b/tests/test_files/enum_cross_module/EnumConsumer.hpp index c25c459..78be000 100644 --- a/tests/test_files/enum_cross_module/EnumConsumer.hpp +++ b/tests/test_files/enum_cross_module/EnumConsumer.hpp @@ -35,4 +35,29 @@ class TaskRunner { } }; +/** + * StatusTracker: Tests cross-module getter→setter roundtrip. + * + * This class lives in EnumConsumer module but uses enums from EnumProvider + * for BOTH getter AND setter methods. This tests the scenario where: + * tracker.setStatus(tracker.getStatus()) + * must work correctly across module boundaries. + */ +class StatusTracker { +public: + StatusTracker() : status_(Task::TaskStatus::PENDING), priority_(Priority::LOW) {} + + // Getter and setter for class-attached enum (Task::TaskStatus) + void setStatus(Task::TaskStatus s) { status_ = s; } + Task::TaskStatus getStatus() const { return status_; } + + // Getter and setter for standalone enum (Priority) + void setPriority(Priority p) { priority_ = p; } + Priority getPriority() const { return priority_; } + +private: + Task::TaskStatus status_; + Priority priority_; +}; + #endif // ENUM_CONSUMER_HPP diff --git a/tests/test_files/enum_cross_module/EnumConsumer.pxd b/tests/test_files/enum_cross_module/EnumConsumer.pxd index b465bec..d40e2d8 100644 --- a/tests/test_files/enum_cross_module/EnumConsumer.pxd +++ b/tests/test_files/enum_cross_module/EnumConsumer.pxd @@ -16,3 +16,13 @@ cdef extern from "EnumConsumer.hpp": bool isCompleted(Task_TaskStatus s) Priority getDefaultPriority() Task_TaskStatus getDefaultStatus() + + # StatusTracker: Tests cross-module getter→setter roundtrip + # This class has getter AND setter for enums from EnumProvider, + # testing that tracker.setStatus(tracker.getStatus()) works. + cdef cppclass StatusTracker: + StatusTracker() + void setStatus(Task_TaskStatus s) + Task_TaskStatus getStatus() + void setPriority(Priority p) + Priority getPriority()