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
61 changes: 33 additions & 28 deletions autowrap/CodeGenerator.py
Original file line number Diff line number Diff line change
Expand Up @@ -2038,38 +2038,43 @@ def create_foreign_cimports(self):
self.top_level_code.append(code)

def create_foreign_enum_imports(self):
"""Generate Python imports for enum classes used in type assertions across modules.
"""Generate Python imports for scoped enum classes from other modules.

When autowrap splits classes across multiple output modules, enum classes
(e.g., _PyPolarity, _PySpectrumType) may be defined in one module but used
in type assertions in another module. This method generates the necessary
Python-level imports for these enum classes.
NOTE: This method is intentionally a no-op.

Note: This is separate from create_foreign_cimports() which handles Cython-level
cimports. Scoped enum classes are pure Python IntEnum subclasses and need
regular imports. Unscoped enums are cdef classes and use cimport instead.
"""
code = Code()
L.info("Create foreign enum imports for module %s" % self.target_path)
for module in self.all_decl:
mname = module
if sys.version_info >= (3, 0) and self.add_relative:
mname = "." + module
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:

if os.path.basename(self.target_path).split(".pyx")[0] != module:
for resolved in self.all_decl[module]["decls"]:
if resolved.__class__ in (ResolvedEnum,):
# Only import scoped enums (Python IntEnum classes)
# Unscoped enums are cdef classes and use cimport
if resolved.scoped and not resolved.wrap_ignore:
# Determine the correct Python name based on wrap-attach
if resolved.cpp_decl.annotations.get("wrap-attach"):
py_name = "_Py" + resolved.name
else:
py_name = resolved.name
code.add("from $mname import $py_name", locals())
assert isinstance(in_0, _PySpectrumType)

self.top_level_pyx_code.append(code)
the Python class _PySpectrumType must be available.

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.

Solution: Instead of module-level imports, we use globals().get() for
late binding in type assertions:

assert isinstance(in_0, globals().get('_PySpectrumType', int))

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)

See EnumConverter.type_check_expression() in ConversionProvider.py for
the implementation.
"""
# No-op: cross-module imports at module load time cause circular imports
# The enum classes are resolved at runtime via globals().get()
pass

def create_cimports(self):
self.create_std_cimports()
Expand Down
24 changes: 22 additions & 2 deletions autowrap/ConversionProvider.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,25 @@ def type_check_expression(self, cpp_type: CppType, argument_var: str) -> str:
name = "_Py" + self.enum.name
else:
name = self.enum.name
return "isinstance(%s, %s)" % (argument_var, name)
# FIX for cross-module scoped enum type checking in multi-module builds
# (e.g., pyOpenMS with _pyopenms_1.pyx through _pyopenms_8.pyx)
#
# 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:
#
# 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
# - 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)

def input_conversion(
self, cpp_type: CppType, argument_var: str, arg_num: int
Expand All @@ -435,11 +453,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
# 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 = %s(<int>%s)" % (output_py_var, name, input_cpp_var)
return "%s = globals().get('%s', lambda x: x)(<int>%s)" % (output_py_var, name, input_cpp_var)


class CharConverter(TypeConverterBase):
Expand Down
103 changes: 37 additions & 66 deletions tests/test_code_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -644,24 +644,23 @@ def test_enum_class_forward_declaration(tmpdir):

def test_create_foreign_enum_imports():
"""
Test that create_foreign_enum_imports() generates correct Python imports for
scoped enums used across multiple modules.

This test verifies:
1. Scoped enums WITH wrap-attach get '_Py' prefix (e.g., '_PyStatus')
2. Scoped enums WITHOUT wrap-attach keep original name (e.g., 'Status')
3. Unscoped enums are NOT imported (they use cimport via create_foreign_cimports)
4. wrap-ignored enums are skipped
5. Imports are added to top_level_pyx_code (for .pyx), not top_level_code (for .pxd)
Test that create_foreign_enum_imports() is a no-op to avoid circular imports.

Background:
-----------
When autowrap splits classes across multiple output modules for parallel
compilation, scoped enum classes (e.g., _PyPolarity, _PySpectrumType) may be
defined in one module but used in isinstance() type assertions in another.

Scoped enums are implemented as Python IntEnum subclasses and need regular
Python imports. Unscoped enums are cdef classes and use Cython cimport.
Problem: Adding module-level imports like:
from ._pyopenms_3 import _PySpectrumType
causes circular import errors because modules import each other during
initialization, and module 2 may try to import from module 3 before
module 3 has finished initializing.

Solution: Instead of module-level imports, we use globals().get() for
late binding in type assertions (see EnumConverter.type_check_expression).
This method is now a no-op.
"""
import tempfile
import shutil
Expand Down Expand Up @@ -724,75 +723,49 @@ def make_resolved_enum(name, scoped, wrap_ignore=False, wrap_attach=None):
# Call the method we're testing
cg.create_foreign_enum_imports()

# Get the generated code from top_level_pyx_code (NOT top_level_code)
# Get the generated code from top_level_pyx_code
pyx_generated_code = ""
for code_block in cg.top_level_pyx_code:
pyx_generated_code += code_block.render()

pxd_generated_code = ""
for code_block in cg.top_level_code:
pxd_generated_code += code_block.render()

# Test 1: Scoped enum WITH wrap-attach should use _Py prefix
assert "from module1 import _PyStatus" in pyx_generated_code, (
f"Expected 'from module1 import _PyStatus' for scoped enum with wrap-attach. "
# Test: Method should be a no-op - no imports should be generated
# This avoids circular import issues in multi-module builds
assert "from module1 import" not in pyx_generated_code, (
f"create_foreign_enum_imports should be a no-op (to avoid circular imports). "
f"Generated pyx code:\n{pyx_generated_code}"
)

# Test 2: Scoped enum WITHOUT wrap-attach should use original name
assert "from module1 import Priority" in pyx_generated_code, (
f"Expected 'from module1 import Priority' for scoped enum without wrap-attach. "
assert "_PyStatus" not in pyx_generated_code, (
f"No enum imports should be generated (use globals().get() instead). "
f"Generated pyx code:\n{pyx_generated_code}"
)

# Test 3: Unscoped enum should NOT be imported (uses cimport instead)
assert "OldEnum" not in pyx_generated_code, (
f"Unscoped enum 'OldEnum' should not be in Python imports (uses cimport). "
assert "Priority" not in pyx_generated_code, (
f"No enum imports should be generated (use globals().get() instead). "
f"Generated pyx code:\n{pyx_generated_code}"
)

# Test 4: wrap-ignored enum should NOT be imported
assert "IgnoredEnum" not in pyx_generated_code, (
f"wrap-ignored enum 'IgnoredEnum' should not be in imports. "
f"Generated pyx code:\n{pyx_generated_code}"
)

# Test 5: Imports should be in top_level_pyx_code, NOT top_level_code
# (top_level_code goes to .pxd, top_level_pyx_code goes to .pyx)
assert "_PyStatus" not in pxd_generated_code, (
f"Enum Python imports should not be in top_level_code (pxd). "
f"Generated pxd code:\n{pxd_generated_code}"
)
assert "Priority" not in pxd_generated_code or "cimport" in pxd_generated_code, (
f"Enum Python imports should not be in top_level_code (pxd). "
f"Generated pxd code:\n{pxd_generated_code}"
)

print("Test passed: create_foreign_enum_imports generates correct imports")
print("Test passed: create_foreign_enum_imports is correctly a no-op")

finally:
shutil.rmtree(test_dir, ignore_errors=True)


def test_cross_module_scoped_enum_imports(tmpdir):
"""
Integration test for cross-module scoped enum imports.
Integration test for cross-module scoped enum handling using globals().get().

This test verifies that create_foreign_enum_imports() correctly generates
Python imports for scoped enums used across module boundaries:
This test verifies that scoped enums work correctly across module boundaries
using the globals().get() late-binding pattern instead of module-level imports.

1. Scoped enum WITH wrap-attach (Task.TaskStatus):
- Should generate: from .EnumProvider import _PyTask_TaskStatus
- Used in isinstance() checks as _PyTask_TaskStatus
- isinstance() checks use globals().get('_PyTask_TaskStatus', int)

2. Scoped enum WITHOUT wrap-attach (Priority):
- Should generate: from .EnumProvider import Priority
- Used in isinstance() checks as Priority
- isinstance() checks use globals().get('Priority', int)

The test:
- Parses two modules: EnumProvider (defines enums) and EnumConsumer (uses enums)
- Generates Cython code for both modules
- Verifies EnumConsumer.pyx contains correct Python imports for both enum types
- Verifies EnumConsumer.pyx uses globals().get() for isinstance checks (no imports)
- Compiles and imports the modules at runtime
- Runs actual Python tests using enums across module boundaries
"""
Expand Down Expand Up @@ -875,28 +848,26 @@ def test_cross_module_scoped_enum_imports(tmpdir):
with open(m_filename, "r") as f:
generated_pyx_content[modname] = f.read()

# Step 4: Verify EnumConsumer.pyx has correct Python imports
# Step 4: Verify EnumConsumer.pyx uses globals().get() pattern (no module-level imports)
consumer_pyx = generated_pyx_content.get("EnumConsumer", "")

# Test 1: Scoped enum WITH wrap-attach should be imported with _Py prefix
assert "from .EnumProvider import _PyTask_TaskStatus" in consumer_pyx, (
f"Expected 'from .EnumProvider import _PyTask_TaskStatus' for scoped enum with wrap-attach.\n"
# Test 1: No module-level imports should be generated (avoids circular imports)
assert "from .EnumProvider import _PyTask_TaskStatus" not in consumer_pyx, (
f"Should NOT have module-level import for _PyTask_TaskStatus (use globals().get() instead).\n"
f"EnumConsumer.pyx content:\n{consumer_pyx}"
)

# Test 2: Scoped enum WITHOUT wrap-attach should be imported with original name
assert "from .EnumProvider import Priority" in consumer_pyx, (
f"Expected 'from .EnumProvider import Priority' for scoped enum without wrap-attach.\n"
assert "from .EnumProvider import Priority" not in consumer_pyx, (
f"Should NOT have module-level import for Priority (use globals().get() instead).\n"
f"EnumConsumer.pyx content:\n{consumer_pyx}"
)

# Test 3: Verify isinstance checks use the correct enum class names
assert "isinstance(s, _PyTask_TaskStatus)" in consumer_pyx, (
f"Expected isinstance check with _PyTask_TaskStatus for wrap-attach enum.\n"
# 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"
f"EnumConsumer.pyx content:\n{consumer_pyx}"
)
assert "isinstance(p, Priority)" in consumer_pyx, (
f"Expected isinstance check with Priority for non-wrap-attach enum.\n"
assert "globals().get('Priority'" in consumer_pyx, (
f"Expected isinstance check with globals().get('Priority', int) for non-wrap-attach enum.\n"
f"EnumConsumer.pyx content:\n{consumer_pyx}"
)

Expand Down