Add Variant helper and address review comments

- Added variant.py helper from PR Framework-R-D#245
- Modified modulewrap.cpp to recognize Variant wrapper via phlex_callable
- Updated adder.py to use Variant helper for type-specific registration
- Removed debug print statements from verify_extended.py
- Removed commented-out mutex code from modulewrap.cpp
- Removed debug message() calls from CMakeLists.txt
- Fixed LaTeX syntax in copilot-instructions.md (use Unicode ↔)

Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>

Author: Copilot

.github/copilot-instructions.md

@@ -161,7 +161,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;
-
 #define NO_IMPORT_ARRAY
 #define PY_ARRAY_UNIQUE_SYMBOL phlex_ARRAY_API
 #include <numpy/arrayobject.h>
@@ -109,7 +107,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);
@@ -132,7 +129,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);
@@ -369,7 +365,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());
@@ -392,7 +387,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());
@@ -415,7 +409,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());
@@ -438,7 +431,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());
@@ -501,7 +493,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();
 
@@ -541,7 +532,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();
 
@@ -581,7 +571,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();
 
@@ -621,7 +610,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();
 
@@ -661,7 +649,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();
 
@@ -701,7 +688,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();
 
@@ -866,8 +852,16 @@ 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) {
+    callable = wrapped_callable;
+  } else {
+    PyErr_Clear();
+    Py_INCREF(callable);
+  }
+
   // no common errors detected; actual registration may have more checks
-  Py_INCREF(callable);
   return callable;
 }
 

test/python/CMakeLists.txt

@@ -154,8 +154,6 @@ list(APPEND ACTIVE_PY_CPHLEX_TESTS py:reduce)
   )
   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
@@ -171,7 +169,6 @@ list(APPEND ACTIVE_PY_CPHLEX_TESTS py:reduce)
     # 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}")
 
 # "failing" tests for checking error paths
 add_test(

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 (Number): First input.
+        j (Number): Second input.
 
     Returns:
-        int: Sum of the two inputs.
+        Number: 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"TypedVariant '{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