diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b63964d2c..8cf50e421 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -143,7 +143,7 @@ All Markdown files must strictly follow these markdownlint rules: - **C++ Driver**: Provides data streams (e.g., `test/python/driver.cpp`). - **Jsonnet Config**: Wires the graph (e.g., `test/python/pytypes.jsonnet`). - **Python Script**: Implements algorithms (e.g., `test/python/test_types.py`). -- **Type Conversion**: `plugins/python/src/modulewrap.cpp` handles C++ $\leftrightarrow$ Python conversion. +- **Type Conversion**: `plugins/python/src/modulewrap.cpp` handles C++ ↔ Python conversion. - **Mechanism**: Uses string comparison of type names (e.g., `"float64]]"`). This is brittle. - **Requirement**: Ensure converters exist for all types used in tests (e.g., `float`, `double`, `unsigned int`, and their vector equivalents). - **Warning**: Exact type matches are required. `numpy.float32` != `float`. diff --git a/plugins/python/src/modulewrap.cpp b/plugins/python/src/modulewrap.cpp index dd5f3ca38..ccba9c99c 100644 --- a/plugins/python/src/modulewrap.cpp +++ b/plugins/python/src/modulewrap.cpp @@ -9,8 +9,6 @@ #include #include -// static std::mutex g_py_mutex; - #ifdef PHLEX_HAVE_NUMPY #define NO_IMPORT_ARRAY #define PY_ARRAY_UNIQUE_SYMBOL phlex_ARRAY_API @@ -111,7 +109,6 @@ namespace { static_assert(sizeof...(Args) == N, "Argument count mismatch"); PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); PyObject* result = PyObject_CallFunctionObjArgs( (PyObject*)m_callable, lifeline_transform(args.get())..., nullptr); @@ -134,7 +131,6 @@ namespace { static_assert(sizeof...(Args) == N, "Argument count mismatch"); PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); PyObject* result = PyObject_CallFunctionObjArgs((PyObject*)m_callable, (PyObject*)args.get()..., nullptr); @@ -372,7 +368,6 @@ namespace { static PyObjectPtr vint_to_py(std::shared_ptr> const& v) { PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); if (!v) return PyObjectPtr(); PyObject* list = PyList_New(v->size()); @@ -395,7 +390,6 @@ namespace { static PyObjectPtr vuint_to_py(std::shared_ptr> const& v) { PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); if (!v) return PyObjectPtr(); PyObject* list = PyList_New(v->size()); @@ -418,7 +412,6 @@ namespace { static PyObjectPtr vlong_to_py(std::shared_ptr> const& v) { PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); if (!v) return PyObjectPtr(); PyObject* list = PyList_New(v->size()); @@ -441,7 +434,6 @@ namespace { static PyObjectPtr vulong_to_py(std::shared_ptr> const& v) { PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); if (!v) return PyObjectPtr(); PyObject* list = PyList_New(v->size()); @@ -497,7 +489,6 @@ namespace { static std::shared_ptr> py_to_vint(PyObjectPtr pyobj) { PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); auto vec = std::make_shared>(); PyObject* obj = pyobj.get(); @@ -537,7 +528,6 @@ namespace { static std::shared_ptr> py_to_vuint(PyObjectPtr pyobj) { PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); auto vec = std::make_shared>(); PyObject* obj = pyobj.get(); @@ -577,7 +567,6 @@ namespace { static std::shared_ptr> py_to_vlong(PyObjectPtr pyobj) { PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); auto vec = std::make_shared>(); PyObject* obj = pyobj.get(); @@ -617,7 +606,6 @@ namespace { static std::shared_ptr> py_to_vulong(PyObjectPtr pyobj) { PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); auto vec = std::make_shared>(); PyObject* obj = pyobj.get(); @@ -657,7 +645,6 @@ namespace { static std::shared_ptr> py_to_vfloat(PyObjectPtr pyobj) { PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); auto vec = std::make_shared>(); PyObject* obj = pyobj.get(); @@ -697,7 +684,6 @@ namespace { static std::shared_ptr> py_to_vdouble(PyObjectPtr pyobj) { PyGILRAII gil; - // std::lock_guard lock(g_py_mutex); auto vec = std::make_shared>(); PyObject* obj = pyobj.get(); @@ -863,8 +849,18 @@ static PyObject* parse_args(PyObject* args, return nullptr; } + // special case of Phlex Variant wrapper + PyObject* wrapped_callable = PyObject_GetAttrString(callable, "phlex_callable"); + if (wrapped_callable) { + // PyObject_GetAttrString returns a new reference, which we return + callable = wrapped_callable; + } else { + // No wrapper, use the original callable with incremented reference count + PyErr_Clear(); + Py_INCREF(callable); + } + // no common errors detected; actual registration may have more checks - Py_INCREF(callable); return callable; } diff --git a/test/python/CMakeLists.txt b/test/python/CMakeLists.txt index 0600c7ae7..299298a3c 100644 --- a/test/python/CMakeLists.txt +++ b/test/python/CMakeLists.txt @@ -137,8 +137,6 @@ except Exception: ) list(APPEND ACTIVE_PY_CPHLEX_TESTS py:failure) - message(STATUS "Python_SITELIB: ${Python_SITELIB}") - message(STATUS "Python_SITEARCH: ${Python_SITEARCH}") set(TEST_PYTHONPATH ${CMAKE_CURRENT_SOURCE_DIR}) # Always add site-packages to PYTHONPATH for tests, as embedded python might # not find them especially in spack environments where they are in @@ -154,7 +152,6 @@ except Exception: # Keep this for backward compatibility or if it adds something else endif() set(TEST_PYTHONPATH ${TEST_PYTHONPATH}:$ENV{PYTHONPATH}) - message(STATUS "TEST_PYTHONPATH: ${TEST_PYTHONPATH}") set_tests_properties( ${ACTIVE_PY_CPHLEX_TESTS} diff --git a/test/python/adder.py b/test/python/adder.py index 549dcdab9..d7a40d932 100644 --- a/test/python/adder.py +++ b/test/python/adder.py @@ -4,19 +4,33 @@ real. It serves as a "Hello, World" equivalent for running Python code. """ +from typing import Protocol, TypeVar -def add(i: int, j: int) -> int: +from variant import Variant + + +class AddableProtocol[T](Protocol): + """Typer bound for any types that can be added.""" + + def __add__(self, other: T) -> T: # noqa: D105 + ... + + +Addable = TypeVar('Addable', bound=AddableProtocol) + + +def add(i: Addable, j: Addable) -> Addable: """Add the inputs together and return the sum total. Use the standard `+` operator to add the two inputs together to arrive at their total. Args: - i (int): First input. - j (int): Second input. + i (Addable): First input. + j (Addable): Second input. Returns: - int: Sum of the two inputs. + Addable: Sum of the two inputs. Examples: >>> add(1, 2) @@ -40,4 +54,5 @@ def PHLEX_REGISTER_ALGORITHMS(m, config): Returns: None """ - m.transform(add, input_family=config["input"], output_products=config["output"]) + int_adder = Variant(add, {"i": int, "j": int, "return": int}, "iadd") + m.transform(int_adder, input_family=config["input"], output_products=config["output"]) diff --git a/test/python/variant.py b/test/python/variant.py new file mode 100644 index 000000000..f607207c4 --- /dev/null +++ b/test/python/variant.py @@ -0,0 +1,79 @@ +"""Annotation helper for C++ typing variants. + +Python algorithms are generic, like C++ templates, but the Phlex registration +process requires a single unique signature. These helpers generate annotated +functions for registration with the proper C++ types. +""" + +import copy +from typing import Any, Callable + + +class Variant: + """Wrapper to associate custom annotations with a callable. + + This class wraps a callable and provides custom ``__annotations__`` and + ``__name__`` attributes, allowing the same underlying function or callable + object to be registered multiple times with different type annotations. + + By default, the provided callable is kept by reference, but can be cloned + (e.g. for callable instances) if requested. + + Phlex will recognize the "phlex_callable" data member, allowing an unwrap + and thus saving an indirection. To detect performance degradation, the + wrapper is not callable by default. + + Attributes: + phlex_callable (Callable): The underlying callable (public). + __annotations__ (dict): Type information of arguments and return product. + __name__ (str): The name associated with this variant. + + Examples: + >>> def add(i: Number, j: Number) -> Number: + ... return i + j + ... + >>> int_adder = variant(add, {"i": int, "j": int, "return": int}, "iadd") + """ + + def __init__( + self, + f: Callable, + annotations: dict[str, str | type | Any], + name: str, + clone: bool | str = False, + allow_call: bool = False, + ): + """Annotate the callable F. + + Args: + f (Callable): Annotable function. + annotations (dict): Type information of arguments and return product. + name (str): Name to assign to this variant. + clone (bool|str): If True (or "deep"), creates a shallow (deep) copy + of the callable. + allow_call (bool): Allow this wrapper to forward to the callable. + """ + if clone == 'deep': + self.phlex_callable = copy.deepcopy(f) + elif clone: + self.phlex_callable = copy.copy(f) + else: + self.phlex_callable = f + self.__annotations__ = annotations + self.__name__ = name + self._allow_call = allow_call + + def __call__(self, *args, **kwargs): + """Raises an error if called directly. + + Variant instances should not be called directly. The framework should + extract ``phlex_callable`` instead and call that. + + Raises: + AssertionError: To indicate incorrect usage, unless overridden. + """ + assert self._allow_call, ( + f"Variant '{self.__name__}' was called directly. " + f"The framework should extract phlex_callable instead." + ) + return self.phlex_callable(*args, **kwargs) # type: ignore diff --git a/test/python/verify_extended.py b/test/python/verify_extended.py index 22681b2b6..68dc44d0a 100644 --- a/test/python/verify_extended.py +++ b/test/python/verify_extended.py @@ -1,7 +1,5 @@ """Observers to check for various types in tests.""" -import sys - class VerifierInt: """Verify int values.""" @@ -42,7 +40,6 @@ def __init__(self, sum_total: int): def __call__(self, value: "long") -> None: # type: ignore # noqa: F821 """Check if value matches expected sum.""" - print(f"VerifierLong: value={value}, expected={self._sum_total}") assert value == self._sum_total @@ -57,7 +54,6 @@ def __init__(self, sum_total: int): def __call__(self, value: "unsigned long") -> None: # type: ignore # noqa: F722 """Check if value matches expected sum.""" - print(f"VerifierULong: value={value}, expected={self._sum_total}") assert value == self._sum_total @@ -72,7 +68,6 @@ def __init__(self, sum_total: float): def __call__(self, value: "float") -> None: """Check if value matches expected sum.""" - sys.stderr.write(f"VerifierFloat: value={value}, expected={self._sum_total}\n") assert abs(value - self._sum_total) < 1e-5 @@ -87,7 +82,6 @@ def __init__(self, sum_total: float): def __call__(self, value: "double") -> None: # type: ignore # noqa: F821 """Check if value matches expected sum.""" - print(f"VerifierDouble: value={value}, expected={self._sum_total}") assert abs(value - self._sum_total) < 1e-5 @@ -102,7 +96,6 @@ def __init__(self, expected: bool): def __call__(self, value: bool) -> None: """Check if value matches expected.""" - print(f"VerifierBool: value={value}, expected={self._expected}") assert value == self._expected