Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 75 additions & 19 deletions autowrap/CodeGenerator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'):
Copy link
Contributor

Choose a reason for hiding this comment

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

Nope 😄 this will not work generally

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if only pyopenms uses it ;)

| 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()
Expand Down Expand Up @@ -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
Expand Down
22 changes: 9 additions & 13 deletions autowrap/ConversionProvider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -453,13 +449,13 @@ def output_conversion(
return "%s = <int>%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)(<int>%s)" % (output_py_var, name, input_cpp_var)
return "%s = _get_scoped_enum_class('%s', lambda x: x)(<int>%s)" % (output_py_var, name, input_cpp_var)


class CharConverter(TypeConverterBase):
Expand Down
4 changes: 2 additions & 2 deletions autowrap/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__
32 changes: 27 additions & 5 deletions tests/test_code_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
)

Expand Down Expand Up @@ -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:
Expand Down
25 changes: 25 additions & 0 deletions tests/test_files/enum_cross_module/EnumConsumer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 10 additions & 0 deletions tests/test_files/enum_cross_module/EnumConsumer.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -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()