Merge pull request #6 from greenc-FNAL/copilot/implement-variant-helper-in-pull-213

Integrate Variant helper from PR #245 for type-specific Python algorithm registration

Author: greenc-FNAL

.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`.

plugins/python/src/modulewrap.cpp

@@ -9,8 +9,6 @@
 #include <stdexcept>
 #include <vector>
 
-// 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<std::mutex> 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<std::mutex> 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<std::vector<int>> const& v)
   {
     PyGILRAII gil;
-    // std::lock_guard<std::mutex> 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<std::vector<unsigned int>> const& v)
   {
     PyGILRAII gil;
-    // std::lock_guard<std::mutex> 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<std::vector<long>> const& v)
   {
     PyGILRAII gil;
-    // std::lock_guard<std::mutex> 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<std::vector<unsigned long>> const& v)
   {
     PyGILRAII gil;
-    // std::lock_guard<std::mutex> lock(g_py_mutex);
     if (!v)
       return PyObjectPtr();
     PyObject* list = PyList_New(v->size());
@@ -497,7 +489,6 @@ namespace {
   static std::shared_ptr<std::vector<int>> py_to_vint(PyObjectPtr pyobj)
   {
     PyGILRAII gil;
-    // std::lock_guard<std::mutex> lock(g_py_mutex);
     auto vec = std::make_shared<std::vector<int>>();
     PyObject* obj = pyobj.get();
 
@@ -537,7 +528,6 @@ namespace {
   static std::shared_ptr<std::vector<unsigned int>> py_to_vuint(PyObjectPtr pyobj)
   {
     PyGILRAII gil;
-    // std::lock_guard<std::mutex> lock(g_py_mutex);
     auto vec = std::make_shared<std::vector<unsigned int>>();
     PyObject* obj = pyobj.get();
 
@@ -577,7 +567,6 @@ namespace {
   static std::shared_ptr<std::vector<long>> py_to_vlong(PyObjectPtr pyobj)
   {
     PyGILRAII gil;
-    // std::lock_guard<std::mutex> lock(g_py_mutex);
     auto vec = std::make_shared<std::vector<long>>();
     PyObject* obj = pyobj.get();
 
@@ -617,7 +606,6 @@ namespace {
   static std::shared_ptr<std::vector<unsigned long>> py_to_vulong(PyObjectPtr pyobj)
   {
     PyGILRAII gil;
-    // std::lock_guard<std::mutex> lock(g_py_mutex);
     auto vec = std::make_shared<std::vector<unsigned long>>();
     PyObject* obj = pyobj.get();
 
@@ -657,7 +645,6 @@ namespace {
   static std::shared_ptr<std::vector<float>> py_to_vfloat(PyObjectPtr pyobj)
   {
     PyGILRAII gil;
-    // std::lock_guard<std::mutex> lock(g_py_mutex);
     auto vec = std::make_shared<std::vector<float>>();
     PyObject* obj = pyobj.get();
 
@@ -697,7 +684,6 @@ namespace {
   static std::shared_ptr<std::vector<double>> py_to_vdouble(PyObjectPtr pyobj)
   {
     PyGILRAII gil;
-    // std::lock_guard<std::mutex> lock(g_py_mutex);
     auto vec = std::make_shared<std::vector<double>>();
     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;
 }
 

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}

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"])

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

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