diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 7ddbfb2c..ab1225f4 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -137,3 +137,59 @@ All Markdown files must strictly follow these markdownlint rules: - **MD034**: No bare URLs (for example, use a markdown link like `[text](destination)` instead of a plain URL) - **MD036**: Use # headings, not **Bold:** for titles - **MD040**: Always specify code block language (for example, use '```bash', '```python', '```text', etc.) + +## Development & Testing Workflows + +### Build and Test + +- **Environment**: Always source `setup-env.sh` before building or testing. This applies to all environments (Dev Container, local machine, HPC). +- **Configuration**: + - **Presets**: Prefer `CMakePresets.json` workflows (e.g., `cmake --preset default`). + - **Generator**: Prefer `Ninja` over `Makefiles` when available (`-G Ninja`). +- **Build**: + - **Parallelism**: Always use multiple cores. Ninja does this by default. For `make`, use `cmake --build build -j $(nproc)`. +- **Test**: + - **Parallelism**: Run tests in parallel using `ctest -j $(nproc)` or `ctest --parallel `. + - **Selection**: Run specific tests with `ctest -R "regex"` (e.g., `ctest -R "py:*"`). + - **Debugging**: Use `ctest --output-on-failure` to see logs for failed tests. + - **Guard against known or suspected stalling tests**: Use `ctest --test-timeout` to set the per-test time limit (e.g. `90`) for 90s, _vs_ the default of 1500s. + +### Python Integration + +- **Naming**: Avoid naming Python test scripts `types.py` or other names that shadow standard library modules. This causes obscure import errors (e.g., `ModuleNotFoundError: No module named 'numpy'`). +- **PYTHONPATH**: Only include paths that contain user Python modules loaded by Phlex (for example, the source directory and any build output directory that houses generated modules). Do not append system/Spack/venv `site-packages`; `pymodule.cpp` handles CMAKE_PREFIX_PATH and virtual-environment path adjustments. +- **Test Structure**: + - **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++ ↔ Python conversion. + - **Mechanism**: Uses substring matching on type names (for example, `"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`. + +### Coverage Analysis + +- **Tooling**: The project uses LLVM source-based coverage. +- **Requirement**: The `phlex` binary must catch exceptions in `main` to ensure coverage data is flushed to disk even when tests fail/crash. +- **Generation**: + - **CMake Targets**: `coverage-xml`, `coverage-html` (if configured). + - **Manual**: + 1. Run tests with `LLVM_PROFILE_FILE` set (e.g., `export LLVM_PROFILE_FILE="profraw/%m-%p.profraw"`). + 2. Merge profiles: `llvm-profdata merge -sparse profraw/*.profraw -o coverage.profdata`. + 3. Generate report: `llvm-cov show -instr-profile=coverage.profdata -format=html ...` + +### Local GitHub Actions Testing (`act`) + +- **Tool**: Use `act` to run GitHub Actions workflows locally. +- **Configuration**: Ensure `.actrc` exists in the workspace root with the following content to use a compatible runner image: + ```text + -P ubuntu-latest=catthehacker/ubuntu:act-latest + ``` +- **Usage**: + - List jobs: `act -l` + - Run specific job: `act -j ` (e.g., `act -j python-check`) + - Run specific event: `act pull_request` +- **Troubleshooting**: + - **Docker Socket**: `act` requires access to the Docker socket. In dev containers, this may require specific mount configurations or permissions. + - **Artifacts**: `act` creates a `phlex-src` directory (or similar) for checkout. Ensure this is cleaned up or ignored by tools like `mypy`. + diff --git a/.github/workflows/cmake-build.yaml b/.github/workflows/cmake-build.yaml index ae6e390f..f86077c1 100644 --- a/.github/workflows/cmake-build.yaml +++ b/.github/workflows/cmake-build.yaml @@ -181,7 +181,7 @@ jobs: echo "➡️ Running tests..." echo "::group::Running ctest" - if ctest --progress --output-on-failure -j "$(nproc)"; then + if ctest --progress --output-on-failure --test-timeout 90 -j "$(nproc)"; then echo "::endgroup::" echo "✅ All tests passed." else diff --git a/.github/workflows/coverage.yaml b/.github/workflows/coverage.yaml index 946ce586..289fdbc1 100644 --- a/.github/workflows/coverage.yaml +++ b/.github/workflows/coverage.yaml @@ -58,6 +58,7 @@ jobs: file-type: | cpp cmake + python - name: Report detection outcome run: | @@ -161,7 +162,7 @@ jobs: export LLVM_PROFILE_FILE="$PROFILE_ROOT/%m-%p.profraw" echo "::group::Running ctest for coverage" - if ctest --progress --output-on-failure -j "$(nproc)"; then + if ctest --progress --output-on-failure --test-timeout 90 -j "$(nproc)"; then echo "::endgroup::" echo "✅ All tests passed." else diff --git a/.gitignore b/.gitignore index 6376d96d..bae231d5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,23 +1,27 @@ # Build directories build/ +build-cov/ _build/ *.dir/ -phlex-src -phlex-build/ -CMakeCache.txt +/phlex-src/ +/phlex-build/ +/CMakeCache.txt CMakeFiles/ -_deps/ +/_deps/ _codeql_detected_source_root # CMake user-specific presets (not generated by Spack) -CMakeUserPresets.json +/CMakeUserPresets.json # Coverage reports -coverage.xml -coverage.info -coverage-html/ -.coverage-generated/ -.coverage-artifacts/ +/coverage.profdata +/coverage_*.txt +/coverage.xml +/coverage.info +/coverage-html/ +/profraw/ +/.coverage-generated/ +/.coverage-artifacts/ *.gcda *.gcno *.gcov @@ -45,4 +49,4 @@ __pycache__/ .DS_Store # act (local workflow testing) .act-artifacts/ -.secrets \ No newline at end of file +.secrets diff --git a/CMakeLists.txt b/CMakeLists.txt index 59ff4d81..a5547316 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -49,6 +49,11 @@ project(phlex VERSION 0.1.0 LANGUAGES CXX) cet_cmake_env() # ############################################################################## +# Set CI/test timeouts to a conservative value to avoid long stalls in CI. +# Use cache variables so generated CTest/Dart files pick this up when configured. +set(DART_TESTING_TIMEOUT 90 CACHE STRING "Timeout (s) for Dart/CTest runs") +set(CTEST_TEST_TIMEOUT 90 CACHE STRING "Per-test timeout (s) for CTest") + # Make tools available FetchContent_MakeAvailable(Catch2 GSL mimicpp) @@ -70,13 +75,13 @@ add_compile_options( ) if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") - if( - CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "14.1" - AND CMAKE_COMPILER_VERSION VERSION_LESS "15" - ) - # GCC 14.1 issues many false positives re. array-bounds and - # stringop-overflow - add_compile_options(-Wno-array-bounds -Wno-stringop-overflow) + if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "14.1") + if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "15") + add_compile_options(-Wno-stringop-overflow) + endif() + if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "16") + add_compile_options(-Wno-array-bounds) + endif() endif() endif() @@ -108,7 +113,8 @@ if(ENABLE_TSAN) -g -O1 # Ensure no optimizations interfere with TSan - "$<$:-fno-omit-frame-pointer -fno-optimize-sibling-calls>" + "$<$:-fno-omit-frame-pointer>" + "$<$:-fno-optimize-sibling-calls>" ) add_link_options(-fsanitize=thread) else() @@ -130,7 +136,8 @@ if(ENABLE_ASAN) -g -O1 # Ensure no optimizations interfere with ASan - "$<$:-fno-omit-frame-pointer -fno-optimize-sibling-calls>" + "$<$:-fno-omit-frame-pointer>" + "$<$:-fno-optimize-sibling-calls>" ) add_link_options(-fsanitize=address) else() diff --git a/CMakePresets.json b/CMakePresets.json index 79b4a20a..72e580ac 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -4,6 +4,7 @@ "name": "default", "hidden": false, "cacheVariables": { + "PHLEX_USE_FORM": "ON", "CMAKE_EXPORT_COMPILE_COMMANDS": "YES", "CMAKE_CXX_STANDARD": "20", "CMAKE_CXX_STANDARD_REQUIRED": "YES", diff --git a/plugins/python/CMakeLists.txt b/plugins/python/CMakeLists.txt index 2554a6dc..047869d0 100644 --- a/plugins/python/CMakeLists.txt +++ b/plugins/python/CMakeLists.txt @@ -21,3 +21,8 @@ target_link_libraries(pymodule PRIVATE phlex::module Python::Python Python::NumP target_compile_definitions(pymodule PRIVATE NPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION) install(TARGETS pymodule LIBRARY DESTINATION lib) + +install( + DIRECTORY python/phlex + DESTINATION lib/python${Python_VERSION_MAJOR}.${Python_VERSION_MINOR}/site-packages +) diff --git a/plugins/python/README.md b/plugins/python/README.md new file mode 100644 index 00000000..0c40c5d9 --- /dev/null +++ b/plugins/python/README.md @@ -0,0 +1,56 @@ +# Phlex Python Plugin Architecture + +This directory contains the C++ source code for the Phlex Python plugin, which enables Phlex to execute Python code as part of its computation graph. + +## Architecture Overview + +The integration is built on the **Python C API** (not `pybind11`) to maintain strict control over the interpreter lifecycle and memory management. + +### 1. The "Type Bridge" (`modulewrap.cpp`) + +The core of the integration is the type conversion layer in `src/modulewrap.cpp`. This layer is responsible for: +- Converting Phlex `Product` objects (C++) into Python objects (e.g., `PyObject*`, `numpy.ndarray`). +- Converting Python return values back into Phlex `Product` objects. + +**Critical Implementation Detail:** +The type mapping relies on **string comparison** of type names. + +- **Mechanism**: The C++ code checks whether `type_name()` contains `"float64]]"` to identify a 2D array of doubles. +- **Brittleness**: This is a fragile contract. If the type name changes (e.g., `numpy` changes its string representation) or if a user provides a slightly different type (e.g., `float` vs `np.float32`), the bridge may fail. +- **Extension**: When adding support for new types, you must explicitly add converters in `modulewrap.cpp` for both scalar and vector/array versions. + +### 2. Hybrid Configuration + +Phlex uses a hybrid configuration model involving three languages: + +1. **Jsonnet** (`*.jsonnet`): Defines the computation graph structure. It specifies: + - The nodes in the graph. + - The Python module/class to load for specific nodes. + - Configuration parameters passed to the Python object. +2. **C++ Driver**: The executable that: + - Parses the Jsonnet configuration. + - Initializes the Phlex core. + - Loads the Python interpreter and the specified plugin. +3. **Python Code** (`*.py`): Implements the algorithmic logic. + +### 3. Environment & Testing + +Because the Python interpreter is embedded within the C++ application, the runtime environment is critical. + +- **PYTHONPATH**: Must be set correctly to include: + - The build directory (for generated modules). + - The source directory (for user scripts). + - Do not append system/Spack `site-packages`; `pymodule.cpp` adjusts `sys.path` based on `CMAKE_PREFIX_PATH` and active virtual environments. +- **Naming Collisions**: + - **Warning**: Do not name test files `types.py`, `test.py`, `code.py`, or other names that shadow standard library modules. + - **Consequence**: Shadowing can cause obscure failures in internal libraries (e.g., `numpy` failing to import because it tries to import `types` from the standard library but gets your local file instead). + +## Development Guidelines + +1. **Adding New Types**: + - Update `src/modulewrap.cpp` to handle the new C++ type. + - Add a corresponding test case in `test/python/` to verify the round-trip conversion. +2. **Testing**: + - Use `ctest` to run tests. + - Tests are integration tests: they run the full C++ application which loads the Python script. + - Debugging: Use `ctest --output-on-failure` to see Python exceptions. diff --git a/plugins/python/python/phlex/__init__.py b/plugins/python/python/phlex/__init__.py new file mode 100644 index 00000000..6dca6415 --- /dev/null +++ b/plugins/python/python/phlex/__init__.py @@ -0,0 +1,83 @@ +"""Phlex Python Utilities. + +Call helpers and type annotation tools for the Phlex framework. +""" + +import copy +from typing import Any, Callable + + +class AdjustAnnotations: + """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 = AdjustAnnotations(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 + + # Expose __code__ from the underlying callable if available, to aid + # introspection (e.g. by C++ modulewrap). + self.__code__ = getattr(self.phlex_callable, "__code__", None) + self.__defaults__ = getattr(self.phlex_callable, "__defaults__", None) + self.__kwdefaults__ = getattr(self.phlex_callable, "__kwdefaults__", None) + + def __call__(self, *args, **kwargs): + """Raises an error if called directly. + + AdjustAnnotations 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"AdjustAnnotations '{self.__name__}' was called directly. " + f"The framework should extract phlex_callable instead." + ) + return self.phlex_callable(*args, **kwargs) # type: ignore diff --git a/plugins/python/src/lifelinewrap.cpp b/plugins/python/src/lifelinewrap.cpp index 0f81e6bb..a00e1d72 100644 --- a/plugins/python/src/lifelinewrap.cpp +++ b/plugins/python/src/lifelinewrap.cpp @@ -31,9 +31,14 @@ static int ll_clear(py_lifeline_t* pyobj) static void ll_dealloc(py_lifeline_t* pyobj) { + // This type participates in GC; untrack before clearing references so the + // collector does not traverse a partially torn-down object during dealloc. + PyObject_GC_UnTrack(pyobj); Py_CLEAR(pyobj->m_view); typedef std::shared_ptr generic_shared_t; pyobj->m_source.~generic_shared_t(); + // Use tp_free to pair with tp_alloc for GC-tracked Python objects. + Py_TYPE(pyobj)->tp_free((PyObject*)pyobj); } // clang-format off diff --git a/plugins/python/src/modulewrap.cpp b/plugins/python/src/modulewrap.cpp index 00b123d6..aa1fa215 100644 --- a/plugins/python/src/modulewrap.cpp +++ b/plugins/python/src/modulewrap.cpp @@ -15,6 +15,17 @@ using namespace phlex::experimental; using phlex::concurrency; using phlex::product_query; +struct PyObjectDeleter { + void operator()(PyObject* p) const + { + if (p && Py_IsInitialized()) { + phlex::experimental::PyGILRAII gil; + Py_DECREF(p); + } + } +}; +using PyObjectPtr = std::shared_ptr; + // TODO: the layer is currently hard-wired and should come from the product // specification instead, but that doesn't exist in Python yet. static std::string const LAYER = "job"; @@ -55,12 +66,12 @@ namespace { return oss.str(); } - static inline PyObject* lifeline_transform(intptr_t arg) + static inline PyObject* lifeline_transform(PyObject* arg) { - if (Py_TYPE((PyObject*)arg) == &PhlexLifeline_Type) { + if (Py_TYPE(arg) == &PhlexLifeline_Type) { return ((py_lifeline_t*)arg)->m_view; } - return (PyObject*)arg; + return arg; } // callable object managing the callback @@ -75,27 +86,36 @@ namespace { } py_callback(py_callback const& pc) { + PyGILRAII gil; Py_INCREF(pc.m_callable); m_callable = pc.m_callable; } py_callback& operator=(py_callback const& pc) { if (this != &pc) { + PyGILRAII gil; Py_INCREF(pc.m_callable); m_callable = pc.m_callable; } + return *this; + } + ~py_callback() + { + if (Py_IsInitialized()) { + PyGILRAII gil; + Py_DECREF(m_callable); + } } - ~py_callback() { Py_DECREF(m_callable); } template - intptr_t call(Args... args) + PyObjectPtr call(Args... args) { static_assert(sizeof...(Args) == N, "Argument count mismatch"); PyGILRAII gil; - PyObject* result = - PyObject_CallFunctionObjArgs((PyObject*)m_callable, lifeline_transform(args)..., nullptr); + PyObject* result = PyObject_CallFunctionObjArgs( + (PyObject*)m_callable, lifeline_transform(args.get())..., nullptr); std::string error_msg; if (!result) { @@ -103,12 +123,11 @@ namespace { error_msg = "Unknown python error"; } - decref_all(args...); - - if (!error_msg.empty()) + if (!error_msg.empty()) { throw std::runtime_error(error_msg.c_str()); + } - return (intptr_t)result; + return PyObjectPtr(result, PyObjectDeleter()); } template @@ -119,7 +138,7 @@ namespace { PyGILRAII gil; PyObject* result = - PyObject_CallFunctionObjArgs((PyObject*)m_callable, (PyObject*)args..., nullptr); + PyObject_CallFunctionObjArgs((PyObject*)m_callable, (PyObject*)args.get()..., nullptr); std::string error_msg; if (!result) { @@ -128,48 +147,42 @@ namespace { } else Py_DECREF(result); - decref_all(args...); - - if (!error_msg.empty()) + if (!error_msg.empty()) { throw std::runtime_error(error_msg.c_str()); - } - - private: - template - void decref_all(Args... args) - { - // helper to decrement reference counts of N arguments - (Py_DECREF((PyObject*)args), ...); + } } }; // use explicit instatiations to ensure that the function signature can // be derived by the graph builder struct py_callback_1 : public py_callback<1> { - intptr_t operator()(intptr_t arg0) { return call(arg0); } + PyObjectPtr operator()(PyObjectPtr arg0) { return call(arg0); } }; struct py_callback_2 : public py_callback<2> { - intptr_t operator()(intptr_t arg0, intptr_t arg1) { return call(arg0, arg1); } + PyObjectPtr operator()(PyObjectPtr arg0, PyObjectPtr arg1) { return call(arg0, arg1); } }; struct py_callback_3 : public py_callback<3> { - intptr_t operator()(intptr_t arg0, intptr_t arg1, intptr_t arg2) + PyObjectPtr operator()(PyObjectPtr arg0, PyObjectPtr arg1, PyObjectPtr arg2) { return call(arg0, arg1, arg2); } }; struct py_callback_1v : public py_callback<1> { - void operator()(intptr_t arg0) { callv(arg0); } + void operator()(PyObjectPtr arg0) { callv(arg0); } }; struct py_callback_2v : public py_callback<2> { - void operator()(intptr_t arg0, intptr_t arg1) { callv(arg0, arg1); } + void operator()(PyObjectPtr arg0, PyObjectPtr arg1) { callv(arg0, arg1); } }; struct py_callback_3v : public py_callback<3> { - void operator()(intptr_t arg0, intptr_t arg1, intptr_t arg2) { callv(arg0, arg1, arg2); } + void operator()(PyObjectPtr arg0, PyObjectPtr arg1, PyObjectPtr arg2) + { + callv(arg0, arg1, arg2); + } }; static std::vector cseq(PyObject* coll) @@ -207,6 +220,16 @@ namespace { std::string ann; if (!PyUnicode_Check(pyobj)) { PyObject* pystr = PyObject_GetAttrString(pyobj, "__name__"); // eg. for classes + + // generics like Union have a __name__ that is not useful for our purposes + if (pystr) { + char const* cstr = PyUnicode_AsUTF8(pystr); + if (cstr && (strcmp(cstr, "Union") == 0 || strcmp(cstr, "Optional") == 0)) { + Py_DECREF(pystr); + pystr = nullptr; + } + } + if (!pystr) { PyErr_Clear(); pystr = PyObject_Str(pyobj); @@ -220,7 +243,7 @@ namespace { // for numpy typing, there's no useful way of figuring out the type from the // name of the type, only from its string representation, so fall through and // let this method return str() - if (ann != "ndarray") + if (ann != "ndarray" && ann != "list") return ann; // start over for numpy type using result from str() @@ -279,7 +302,7 @@ namespace { unsigned long ul = PyLong_AsUnsignedLong(pyobject); if (ul == (unsigned long)-1 && PyErr_Occurred() && PyLong_Check(pyobject)) { PyErr_Clear(); - long i = PyLong_AS_LONG(pyobject); + long i = PyLong_AsLong(pyobject); if (0 <= i) { ul = (unsigned long)i; } else { @@ -292,17 +315,35 @@ namespace { } #define BASIC_CONVERTER(name, cpptype, topy, frompy) \ - static intptr_t name##_to_py(cpptype a) \ + static PyObjectPtr name##_to_py(cpptype a) \ { \ PyGILRAII gil; \ - return (intptr_t)topy(a); \ + return PyObjectPtr(topy(a), PyObjectDeleter()); \ } \ \ - static cpptype py_to_##name(intptr_t pyobj) \ + static cpptype py_to_##name(PyObjectPtr pyobj) \ { \ PyGILRAII gil; \ - cpptype i = (cpptype)frompy((PyObject*)pyobj); \ - Py_DECREF((PyObject*)pyobj); \ + cpptype i = (cpptype)frompy(pyobj.get()); \ + if (PyErr_Occurred()) { \ + PyObject *ptype, *pvalue, *ptraceback; \ + PyErr_Fetch(&ptype, &pvalue, &ptraceback); \ + PyErr_NormalizeException(&ptype, &pvalue, &ptraceback); \ + std::string msg = "Python conversion error for type " #name; \ + if (pvalue) { \ + PyObject* pstr = PyObject_Str(pvalue); \ + if (pstr) { \ + msg += ": "; \ + msg += PyUnicode_AsUTF8(pstr); \ + Py_DECREF(pstr); \ + } \ + } \ + Py_XDECREF(ptype); \ + Py_XDECREF(pvalue); \ + Py_XDECREF(ptraceback); \ + throw std::runtime_error(msg); \ + } \ + pyobj.reset(); \ return i; \ } @@ -315,7 +356,7 @@ namespace { BASIC_CONVERTER(double, double, PyFloat_FromDouble, PyFloat_AsDouble) #define VECTOR_CONVERTER(name, cpptype, nptype) \ - static intptr_t name##_to_py(std::shared_ptr> const& v) \ + static PyObjectPtr name##_to_py(std::shared_ptr> const& v) \ { \ PyGILRAII gil; \ \ @@ -330,7 +371,7 @@ namespace { ); \ \ if (!np_view) \ - return (intptr_t)nullptr; \ + return PyObjectPtr(); \ \ /* make the data read-only by not making it writable */ \ PyArray_CLEARFLAGS((PyArrayObject*)np_view, NPY_ARRAY_WRITEABLE); \ @@ -340,10 +381,10 @@ namespace { /* when passing it to the registered Python function */ \ py_lifeline_t* pyll = \ (py_lifeline_t*)PhlexLifeline_Type.tp_new(&PhlexLifeline_Type, nullptr, nullptr); \ + new (&pyll->m_source) std::shared_ptr(v); \ pyll->m_view = np_view; /* steals reference */ \ - pyll->m_source = v; \ \ - return (intptr_t)pyll; \ + return PyObjectPtr((PyObject*)pyll, PyObjectDeleter()); \ } VECTOR_CONVERTER(vint, int, NPY_INT) @@ -354,20 +395,19 @@ namespace { VECTOR_CONVERTER(vdouble, double, NPY_DOUBLE) #define NUMPY_ARRAY_CONVERTER(name, cpptype, nptype) \ - static std::shared_ptr> py_to_##name(intptr_t pyobj) \ + static std::shared_ptr> py_to_##name(PyObjectPtr pyobj) \ { \ PyGILRAII gil; \ \ auto vec = std::make_shared>(); \ \ /* TODO: because of unresolved ownership issues, copy the full array contents */ \ - if (!pyobj || !PyArray_Check((PyObject*)pyobj)) { \ + if (!pyobj || !PyArray_Check(pyobj.get())) { \ PyErr_Clear(); /* how to report an error? */ \ - Py_DECREF((PyObject*)pyobj); \ return vec; \ } \ \ - PyArrayObject* arr = (PyArrayObject*)pyobj; \ + PyArrayObject* arr = (PyArrayObject*)pyobj.get(); \ \ /* TODO: flattening the array here seems to be the only workable solution */ \ npy_intp* dims = PyArray_DIMS(arr); \ @@ -381,19 +421,341 @@ namespace { vec->reserve(total); \ vec->insert(vec->end(), raw, raw + total); \ \ - Py_DECREF((PyObject*)pyobj); \ return vec; \ } - NUMPY_ARRAY_CONVERTER(vint, int, NPY_INT) - NUMPY_ARRAY_CONVERTER(vuint, unsigned int, NPY_UINT) - NUMPY_ARRAY_CONVERTER(vlong, long, NPY_LONG) - NUMPY_ARRAY_CONVERTER(vulong, unsigned long, NPY_ULONG) - NUMPY_ARRAY_CONVERTER(vfloat, float, NPY_FLOAT) - NUMPY_ARRAY_CONVERTER(vdouble, double, NPY_DOUBLE) + static std::shared_ptr> py_to_vint(PyObjectPtr pyobj) + { + PyGILRAII gil; + auto vec = std::make_shared>(); + PyObject* obj = pyobj.get(); + + if (obj) { + if (PyList_Check(obj)) { + size_t size = PyList_Size(obj); + vec->reserve(size); + for (size_t i = 0; i < size; ++i) { + PyObject* item = PyList_GetItem(obj, i); + if (!item) { + PyErr_Print(); + break; + } + long val = PyLong_AsLong(item); + if (PyErr_Occurred()) { + PyErr_Print(); + break; + } + vec->push_back((int)val); + } + } else if (PyArray_Check(obj)) { + PyArrayObject* arr = (PyArrayObject*)obj; + npy_intp* dims = PyArray_DIMS(arr); + int nd = PyArray_NDIM(arr); + size_t total = 1; + for (int i = 0; i < nd; ++i) + total *= static_cast(dims[i]); + + int* raw = static_cast(PyArray_DATA(arr)); + vec->reserve(total); + vec->insert(vec->end(), raw, raw + total); + } + } + return vec; + } + static std::shared_ptr> py_to_vuint(PyObjectPtr pyobj) + { + PyGILRAII gil; + auto vec = std::make_shared>(); + PyObject* obj = pyobj.get(); + + if (obj) { + if (PyList_Check(obj)) { + size_t size = PyList_Size(obj); + vec->reserve(size); + for (size_t i = 0; i < size; ++i) { + PyObject* item = PyList_GetItem(obj, i); + if (!item) { + PyErr_Print(); + break; + } + unsigned long val = PyLong_AsUnsignedLong(item); + if (PyErr_Occurred()) { + PyErr_Print(); + break; + } + vec->push_back((unsigned int)val); + } + } else if (PyArray_Check(obj)) { + PyArrayObject* arr = (PyArrayObject*)obj; + npy_intp* dims = PyArray_DIMS(arr); + int nd = PyArray_NDIM(arr); + size_t total = 1; + for (int i = 0; i < nd; ++i) + total *= static_cast(dims[i]); + + unsigned int* raw = static_cast(PyArray_DATA(arr)); + vec->reserve(total); + vec->insert(vec->end(), raw, raw + total); + } + } + return vec; + } + static std::shared_ptr> py_to_vlong(PyObjectPtr pyobj) + { + PyGILRAII gil; + auto vec = std::make_shared>(); + PyObject* obj = pyobj.get(); + + if (obj) { + if (PyList_Check(obj)) { + size_t size = PyList_Size(obj); + vec->reserve(size); + for (size_t i = 0; i < size; ++i) { + PyObject* item = PyList_GetItem(obj, i); + if (!item) { + PyErr_Print(); + break; + } + long val = PyLong_AsLong(item); + if (PyErr_Occurred()) { + PyErr_Print(); + break; + } + vec->push_back(val); + } + } else if (PyArray_Check(obj)) { + PyArrayObject* arr = (PyArrayObject*)obj; + npy_intp* dims = PyArray_DIMS(arr); + int nd = PyArray_NDIM(arr); + size_t total = 1; + for (int i = 0; i < nd; ++i) + total *= static_cast(dims[i]); + + long* raw = static_cast(PyArray_DATA(arr)); + vec->reserve(total); + vec->insert(vec->end(), raw, raw + total); + } + } + return vec; + } + static std::shared_ptr> py_to_vulong(PyObjectPtr pyobj) + { + PyGILRAII gil; + auto vec = std::make_shared>(); + PyObject* obj = pyobj.get(); + + if (obj) { + if (PyList_Check(obj)) { + size_t size = PyList_Size(obj); + vec->reserve(size); + for (size_t i = 0; i < size; ++i) { + PyObject* item = PyList_GetItem(obj, i); + if (!item) { + PyErr_Print(); + break; + } + unsigned long val = PyLong_AsUnsignedLong(item); + if (PyErr_Occurred()) { + PyErr_Print(); + break; + } + vec->push_back(val); + } + } else if (PyArray_Check(obj)) { + PyArrayObject* arr = (PyArrayObject*)obj; + npy_intp* dims = PyArray_DIMS(arr); + int nd = PyArray_NDIM(arr); + size_t total = 1; + for (int i = 0; i < nd; ++i) + total *= static_cast(dims[i]); + + unsigned long* raw = static_cast(PyArray_DATA(arr)); + vec->reserve(total); + vec->insert(vec->end(), raw, raw + total); + } + } + return vec; + } + static std::shared_ptr> py_to_vfloat(PyObjectPtr pyobj) + { + PyGILRAII gil; + auto vec = std::make_shared>(); + PyObject* obj = pyobj.get(); + + if (obj) { + if (PyList_Check(obj)) { + size_t size = PyList_Size(obj); + vec->reserve(size); + for (size_t i = 0; i < size; ++i) { + PyObject* item = PyList_GetItem(obj, i); + if (!item) { + PyErr_Print(); + break; + } + double val = PyFloat_AsDouble(item); + if (PyErr_Occurred()) { + PyErr_Print(); + break; + } + vec->push_back((float)val); + } + } else if (PyArray_Check(obj)) { + PyArrayObject* arr = (PyArrayObject*)obj; + npy_intp* dims = PyArray_DIMS(arr); + int nd = PyArray_NDIM(arr); + size_t total = 1; + for (int i = 0; i < nd; ++i) + total *= static_cast(dims[i]); + + float* raw = static_cast(PyArray_DATA(arr)); + vec->reserve(total); + vec->insert(vec->end(), raw, raw + total); + } + } + return vec; + } + static std::shared_ptr> py_to_vdouble(PyObjectPtr pyobj) + { + PyGILRAII gil; + auto vec = std::make_shared>(); + PyObject* obj = pyobj.get(); + + if (obj) { + if (PyList_Check(obj)) { + size_t size = PyList_Size(obj); + vec->reserve(size); + for (size_t i = 0; i < size; ++i) { + PyObject* item = PyList_GetItem(obj, i); + if (!item) { + PyErr_Print(); + break; + } + double val = PyFloat_AsDouble(item); + if (PyErr_Occurred()) { + PyErr_Print(); + break; + } + vec->push_back(val); + } + } else if (PyArray_Check(obj)) { + PyArrayObject* arr = (PyArrayObject*)obj; + npy_intp* dims = PyArray_DIMS(arr); + int nd = PyArray_NDIM(arr); + size_t total = 1; + for (int i = 0; i < nd; ++i) + total *= static_cast(dims[i]); + + double* raw = static_cast(PyArray_DATA(arr)); + vec->reserve(total); + vec->insert(vec->end(), raw, raw + total); + } + } + return vec; + } } // unnamed namespace +// Helper class to extract annotations in argument definition order. +// +// Rationale: +// The __annotations__ dictionary does not guarantee any iteration order +// relative to the function arguments (especially in older Python versions +// or for certain callable types). Iterating blindly over __annotations__ +// can yield input types in a permuted order, causing Phlex to bind +// C++ inputs to the wrong Python arguments (e.g. matching an 'int' product +// to a 'float' argument). +// +// This class attempts to retrieve the bytecode object (__code__) to access +// co_varnames, which provides the authoritative argument order. It falls +// back to dictionary iteration only if introspection fails. +// +// This logic mirrors the Python test class variant.py originally from PR #245. +class AdjustAnnotations { + PyObject* m_callable; + PyObject* m_annotations; + +public: + AdjustAnnotations(PyObject* callable) : m_callable(callable), m_annotations(nullptr) + { + PyObject* name = PyUnicode_FromString("__annotations__"); + m_annotations = PyObject_GetAttr(m_callable, name); + if (!m_annotations) { + PyErr_Clear(); + // the callable may be an instance with a __call__ method + PyObject* call = PyObject_GetAttrString(m_callable, "__call__"); + if (call) { + m_annotations = PyObject_GetAttr(call, name); + Py_DECREF(call); + } + } + Py_DECREF(name); + } + + ~AdjustAnnotations() { Py_XDECREF(m_annotations); } + + void get_input_types(std::vector& types) + { + if (!m_annotations || !PyDict_Check(m_annotations)) { + return; + } + + // Try to use the code object to get the argument names in order + PyObject* code = PyObject_GetAttrString(m_callable, "__code__"); + if (!code) { + PyErr_Clear(); + PyObject* call = PyObject_GetAttrString(m_callable, "__call__"); + if (call) { + code = PyObject_GetAttrString(call, "__code__"); + Py_DECREF(call); + } + } + + bool found = false; + if (code) { + PyObject* varnames = PyObject_GetAttrString(code, "co_varnames"); + PyObject* argcount = PyObject_GetAttrString(code, "co_argcount"); + if (varnames && argcount) { + long count = PyLong_AsLong(argcount); + for (long i = 0; i < count; ++i) { + PyObject* name = PyTuple_GetItem(varnames, i); + if (name) { + PyObject* type = PyDict_GetItem(m_annotations, name); + if (type) { + types.push_back(annotation_as_text(type)); + found = true; + } + } + } + } + Py_XDECREF(varnames); + Py_XDECREF(argcount); + Py_DECREF(code); + } + + // Fallback to dictionary iteration if code object was not helpful + if (!found) { + PyObject *key, *val; + Py_ssize_t pos = 0; + while (PyDict_Next(m_annotations, &pos, &key, &val)) { + if (PyUnicode_Check(key) && PyUnicode_CompareWithASCIIString(key, "return") == 0) { + continue; + } + types.push_back(annotation_as_text(val)); + } + } + } + + void get_return_type(std::vector& types) + { + if (m_annotations && PyDict_Check(m_annotations)) { + PyObject* ret = PyDict_GetItemString(m_annotations, "return"); + if (ret) { + types.push_back(annotation_as_text(ret)); + } + } + } +}; + #define INSERT_INPUT_CONVERTER(name, alg, inp) \ mod->ph_module->transform("py" #name "_" + inp + "_" + alg, name##_to_py, concurrency::serial) \ .input_family(product_query{product_specification::create(inp), LAYER}) \ @@ -468,35 +830,9 @@ static PyObject* parse_args(PyObject* args, // retrieve C++ (matching) types from annotations input_types.reserve(input_labels.size()); - PyObject* sann = PyUnicode_FromString("__annotations__"); - PyObject* annot = PyObject_GetAttr(callable, sann); - if (!annot) { - // the callable may be an instance with a __call__ method - PyErr_Clear(); - PyObject* callm = PyObject_GetAttrString(callable, "__call__"); - if (callm) { - annot = PyObject_GetAttr(callm, sann); - Py_DECREF(callm); - } - } - Py_DECREF(sann); - - if (annot && PyDict_Check(annot) && PyDict_Size(annot)) { - PyObject* ret = PyDict_GetItemString(annot, "return"); - if (ret) - output_types.push_back(annotation_as_text(ret)); - - // dictionary is ordered with return last if provide (note: the keys here - // could be used as input labels, instead of the ones from the configuration, - // but that is probably not practical in actual use, so they are ignored) - PyObject* values = PyDict_Values(annot); - for (Py_ssize_t i = 0; i < (PyList_GET_SIZE(values) - (ret ? 1 : 0)); ++i) { - PyObject* item = PyList_GET_ITEM(values, i); - input_types.push_back(annotation_as_text(item)); - } - Py_DECREF(values); - } - Py_XDECREF(annot); + AdjustAnnotations adj(callable); + adj.get_return_type(output_types); + adj.get_input_types(input_types); // ignore None as Python's conventional "void" return, which is meaningless in C++ if (output_types.size() == 1 && output_types[0] == "None") @@ -514,8 +850,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; } @@ -556,32 +902,32 @@ static bool insert_input_converters(py_phlex_module* mod, return false; } - pos += 18; - + std::string suffix = inp_type.substr(pos); std::string py_out = cname + "_" + inp + "py"; - if (inp_type.compare(pos, std::string::npos, "int32]]") == 0) { - mod->ph_module->transform("pyvint_" + inp + "_" + cname, vint_to_py, concurrency::serial) - .input_family(product_query{product_specification::create(inp), LAYER}) - .output_products(py_out); - } else if (inp_type.compare(pos, std::string::npos, "uint32]]") == 0) { + + if (suffix.find("uint32]]") != std::string::npos) { mod->ph_module->transform("pyvuint_" + inp + "_" + cname, vuint_to_py, concurrency::serial) .input_family(product_query{product_specification::create(inp), LAYER}) .output_products(py_out); - } else if (inp_type.compare(pos, std::string::npos, "int64]]") == 0) { // need not be true - mod->ph_module->transform("pyvlong_" + inp + "_" + cname, vlong_to_py, concurrency::serial) + } else if (suffix.find("int32]]") != std::string::npos) { + mod->ph_module->transform("pyvint_" + inp + "_" + cname, vint_to_py, concurrency::serial) .input_family(product_query{product_specification::create(inp), LAYER}) .output_products(py_out); - } else if (inp_type.compare(pos, std::string::npos, "uint64]]") == 0) { // id. + } else if (suffix.find("uint64]]") != std::string::npos) { // id. mod->ph_module ->transform("pyvulong_" + inp + "_" + cname, vulong_to_py, concurrency::serial) .input_family(product_query{product_specification::create(inp), LAYER}) .output_products(py_out); - } else if (inp_type.compare(pos, std::string::npos, "float32]]") == 0) { + } else if (suffix.find("int64]]") != std::string::npos) { // need not be true + mod->ph_module->transform("pyvlong_" + inp + "_" + cname, vlong_to_py, concurrency::serial) + .input_family(product_query{product_specification::create(inp), LAYER}) + .output_products(py_out); + } else if (suffix.find("float32]]") != std::string::npos) { mod->ph_module ->transform("pyvfloat_" + inp + "_" + cname, vfloat_to_py, concurrency::serial) .input_family(product_query{product_specification::create(inp), LAYER}) .output_products(py_out); - } else if (inp_type.compare(pos, std::string::npos, "double64]]") == 0) { + } else if (suffix.find("float64]]") != std::string::npos) { mod->ph_module ->transform("pyvdouble_" + inp + "_" + cname, vdouble_to_py, concurrency::serial) .input_family(product_query{product_specification::create(inp), LAYER}) @@ -590,6 +936,22 @@ static bool insert_input_converters(py_phlex_module* mod, PyErr_Format(PyExc_TypeError, "unsupported array input type \"%s\"", inp_type.c_str()); return false; } + } else if (inp_type == "list[int]") { + std::string py_out = cname + "_" + inp + "py"; + mod->ph_module->transform("pyvint_" + inp + "_" + cname, vint_to_py, concurrency::serial) + .input_family(product_query{product_specification::create(inp), LAYER}) + .output_products(py_out); + } else if (inp_type == "list[float]") { + std::string py_out = cname + "_" + inp + "py"; + mod->ph_module->transform("pyvfloat_" + inp + "_" + cname, vfloat_to_py, concurrency::serial) + .input_family(product_query{product_specification::create(inp), LAYER}) + .output_products(py_out); + } else if (inp_type == "list[double]" || inp_type == "list['double']") { + std::string py_out = cname + "_" + inp + "py"; + mod->ph_module + ->transform("pyvdouble_" + inp + "_" + cname, vdouble_to_py, concurrency::serial) + .input_family(product_query{product_specification::create(inp), LAYER}) + .output_products(py_out); } else { PyErr_Format(PyExc_TypeError, "unsupported input type \"%s\"", inp_type.c_str()); return false; @@ -704,7 +1066,7 @@ static PyObject* md_transform(py_phlex_module* mod, PyObject* args, PyObject* kw ->transform("pyvfloat_" + output + "_" + cname, py_to_vfloat, concurrency::serial) .input_family(product_query{product_specification::create(py_in), LAYER}) .output_products(output); - } else if (output_type.compare(pos, std::string::npos, "double64]]") == 0) { + } else if (output_type.compare(pos, std::string::npos, "float64]]") == 0) { mod->ph_module ->transform("pyvdouble_" + output + "_" + cname, py_to_vdouble, concurrency::serial) .input_family(product_query{product_specification::create(py_in), LAYER}) @@ -713,6 +1075,22 @@ static PyObject* md_transform(py_phlex_module* mod, PyObject* args, PyObject* kw PyErr_Format(PyExc_TypeError, "unsupported array output type \"%s\"", output_type.c_str()); return nullptr; } + } else if (output_type == "list[int]") { + auto py_in = "py" + output + "_" + cname; + mod->ph_module->transform("pyvint_" + output + "_" + cname, py_to_vint, concurrency::serial) + .input_family(product_query{product_specification::create(py_in), LAYER}) + .output_products(output); + } else if (output_type == "list[float]") { + auto py_in = "py" + output + "_" + cname; + mod->ph_module->transform("pyvfloat_" + output + "_" + cname, py_to_vfloat, concurrency::serial) + .input_family(product_query{product_specification::create(py_in), LAYER}) + .output_products(output); + } else if (output_type == "list[double]" || output_type == "list['double']") { + auto py_in = "py" + output + "_" + cname; + mod->ph_module + ->transform("pyvdouble_" + output + "_" + cname, py_to_vdouble, concurrency::serial) + .input_family(product_query{product_specification::create(py_in), LAYER}) + .output_products(output); } else { PyErr_Format(PyExc_TypeError, "unsupported output type \"%s\"", output_type.c_str()); return nullptr; diff --git a/scripts/README.md b/scripts/README.md index c21e9167..e70dae24 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -156,12 +156,27 @@ Provides convenient commands for managing code coverage analysis. ```bash # From repository root -./scripts/coverage.sh [COMMAND] [COMMAND...] +./scripts/coverage.sh [--preset ] [COMMAND] [COMMAND...] # Multiple commands in sequence ./scripts/coverage.sh setup test xml html ``` +#### Presets + +The `--preset` flag controls the toolchain and instrumentation method: + +- **`coverage-clang`** (Default): + - Uses LLVM source-based coverage. + - Best for local development (fast, accurate). + - Generates high-fidelity HTML reports. + - Key commands: `setup`, `test`, `html`, `view`, `summary`. + +- **`coverage-gcc`**: + - Uses `gcov` instrumentation. + - Best for CI pipelines requiring XML output (e.g., Codecov). + - Key commands: `setup`, `test`, `xml`, `upload`. + #### Commands | Command | Description | diff --git a/test/python/CMakeLists.txt b/test/python/CMakeLists.txt index 846848ab..aded0ae9 100644 --- a/test/python/CMakeLists.txt +++ b/test/python/CMakeLists.txt @@ -9,15 +9,30 @@ function(check_python_module_version MODULE_NAME MIN_VERSION OUT_VAR) "import sys try: import ${MODULE_NAME} - from packaging.version import parse as parse_version installed_version = getattr(${MODULE_NAME}, '__version__', None) - if parse_version(installed_version) >= parse_version('${MIN_VERSION}'): + if not installed_version: + sys.exit(2) + + def parse(v): + return tuple(map(int, v.split('.')[:3])) + + if parse(installed_version) >= parse('${MIN_VERSION}'): sys.exit(0) else: sys.exit(2) # Version too low -except ImportError: +except ImportError as e: + print(f'ImportError: {e}') + sys.exit(1) +except Exception as e: + print(f'Exception: {e}') sys.exit(1)" RESULT_VARIABLE _module_check_result + OUTPUT_VARIABLE _module_check_out + ERROR_VARIABLE _module_check_err + ) + message( + STATUS + "Check ${MODULE_NAME}: Res=${_module_check_result} Out=${_module_check_out} Err=${_module_check_err}" ) if(_module_check_result EQUAL 0) @@ -53,6 +68,8 @@ if(HAS_CPPYY) set(PYTHON_TEST_PHLEX_INSTALL ${CMAKE_SOURCE_DIR}) endif() + set(PYTHON_TEST_FILES test_phlex.py unit_test_annotations.py) + # Determine pytest command based on coverage support if(HAS_PYTEST_COV AND ENABLE_COVERAGE) set( @@ -64,11 +81,11 @@ if(HAS_CPPYY) --cov-report=term-missing --cov-report=xml:${CMAKE_BINARY_DIR}/coverage-python.xml --cov-report=html:${CMAKE_BINARY_DIR}/coverage-python-html - test_phlex.py + ${PYTHON_TEST_FILES} ) message(STATUS "Python tests will run with coverage reporting (pytest-cov)") else() - set(PYTEST_COMMAND ${PYTHON_TEST_EXECUTABLE} -m pytest test_phlex.py) + set(PYTEST_COMMAND ${PYTHON_TEST_EXECUTABLE} -m pytest ${PYTHON_TEST_FILES}) if(ENABLE_COVERAGE AND NOT HAS_PYTEST_COV) message(WARNING "ENABLE_COVERAGE is ON but pytest-cov not found; Python coverage disabled") endif() @@ -78,18 +95,88 @@ if(HAS_CPPYY) add_test(NAME py:phlex COMMAND ${PYTEST_COMMAND} WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}) set_property(TEST py:phlex PROPERTY ENVIRONMENT "PHLEX_INSTALL=${PYTHON_TEST_PHLEX_INSTALL}") + + if(HAS_PYTEST_COV) + add_custom_target( + coverage-python + COMMAND ${PYTEST_COMMAND} + WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} + COMMENT "Running Python coverage report" + ) + endif() endif() set(ACTIVE_PY_CPHLEX_TESTS "") +# numpy support if installed +if(HAS_NUMPY) + # phlex-based tests that require numpy support + add_test(NAME py:vec COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyvec.jsonnet) + list(APPEND ACTIVE_PY_CPHLEX_TESTS py:vec) + + add_test(NAME py:vectypes COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyvectypes.jsonnet) + list(APPEND ACTIVE_PY_CPHLEX_TESTS py:vectypes) + + add_test(NAME py:callback3 COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pycallback3.jsonnet) + list(APPEND ACTIVE_PY_CPHLEX_TESTS py:callback3) + + # Expect failure for these tests (check for error propagation and type checking) + add_test(NAME py:raise COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyraise.jsonnet) + set_tests_properties( + py:raise + PROPERTIES PASS_REGULAR_EXPRESSION "RuntimeError: Intentional failure" + ) + list(APPEND ACTIVE_PY_CPHLEX_TESTS py:raise) + + add_test(NAME py:badbool COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pybadbool.jsonnet) + set_tests_properties( + py:badbool + PROPERTIES + PASS_REGULAR_EXPRESSION + "Python conversion error for type bool: boolean value should be bool, or integer 1 or 0" + ) + list(APPEND ACTIVE_PY_CPHLEX_TESTS py:badbool) + + add_test(NAME py:badint COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pybadint.jsonnet) + set_tests_properties( + py:badint + PROPERTIES + PASS_REGULAR_EXPRESSION + "Python conversion error for type long: int/long conversion expects an integer object" + ) + list(APPEND ACTIVE_PY_CPHLEX_TESTS py:badint) + + add_test(NAME py:baduint COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pybaduint.jsonnet) + set_tests_properties( + py:baduint + PROPERTIES + PASS_REGULAR_EXPRESSION + "Python conversion error for type uint: can't convert negative value to unsigned long" + ) + list(APPEND ACTIVE_PY_CPHLEX_TESTS py:baduint) + + add_test( + NAME py:mismatch_ann + COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pymismatch_annotations.jsonnet + ) + set_tests_properties( + py:mismatch_ann + PROPERTIES + PASS_REGULAR_EXPRESSION "number of inputs .* does not match number of annotation types" + ) + list(APPEND ACTIVE_PY_CPHLEX_TESTS py:mismatch_ann) + + add_test(NAME py:veclists COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyveclists.jsonnet) + list(APPEND ACTIVE_PY_CPHLEX_TESTS py:veclists) + + add_test(NAME py:types COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pytypes.jsonnet) + list(APPEND ACTIVE_PY_CPHLEX_TESTS py:types) +endif() + # C++ helper to provide a driver add_library(cppsource4py MODULE source.cpp) target_link_libraries(cppsource4py PRIVATE phlex::module) -# phlex-based tests that require numpy support -add_test(NAME py:vec COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyvec.jsonnet) -list(APPEND ACTIVE_PY_CPHLEX_TESTS py:vec) - # phlex-based tests (no cppyy dependency) add_test(NAME py:add COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyadd.jsonnet) list(APPEND ACTIVE_PY_CPHLEX_TESTS py:add) @@ -100,12 +187,24 @@ list(APPEND ACTIVE_PY_CPHLEX_TESTS py:config) add_test(NAME py:reduce COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyreduce.jsonnet) list(APPEND ACTIVE_PY_CPHLEX_TESTS py:reduce) +add_test(NAME py:coverage COMMAND phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pycoverage.jsonnet) +list(APPEND ACTIVE_PY_CPHLEX_TESTS py:coverage) + +add_test( + NAME py:mismatch + COMMAND ${PROJECT_BINARY_DIR}/bin/phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pymismatch.jsonnet +) +set_tests_properties( + py:mismatch + PROPERTIES + PASS_REGULAR_EXPRESSION "number of inputs .* does not match number of annotation types" +) +list(APPEND ACTIVE_PY_CPHLEX_TESTS py:mismatch) + # "failing" tests for checking error paths add_test( NAME py:failure - COMMAND - ${CMAKE_CURRENT_SOURCE_DIR}/failing_test_wrap.sh ${PROJECT_BINARY_DIR}/bin/phlex -c - ${CMAKE_CURRENT_SOURCE_DIR}/pyfailure.jsonnet + COMMAND ${PROJECT_BINARY_DIR}/bin/phlex -c ${CMAKE_CURRENT_SOURCE_DIR}/pyfailure.jsonnet ) set_tests_properties( py:failure @@ -113,8 +212,26 @@ set_tests_properties( ) list(APPEND ACTIVE_PY_CPHLEX_TESTS py:failure) +set(TEST_PYTHONPATH ${CMAKE_CURRENT_SOURCE_DIR}) +# Add the python plugin source directory to PYTHONPATH so tests can use phlex package +set(TEST_PYTHONPATH ${TEST_PYTHONPATH}:${PROJECT_SOURCE_DIR}/plugins/python/python) + +# Always add site-packages to PYTHONPATH for tests, as embedded python might +# not find them especially in spack environments where they are in +# non-standard locations +if(Python_SITELIB) + set(TEST_PYTHONPATH ${TEST_PYTHONPATH}:${Python_SITELIB}) +endif() +if(Python_SITEARCH AND NOT "${Python_SITEARCH}" STREQUAL "${Python_SITELIB}") + set(TEST_PYTHONPATH ${TEST_PYTHONPATH}:${Python_SITEARCH}) +endif() + +if(DEFINED ENV{VIRTUAL_ENV}) + # Keep this for backward compatibility or if it adds something else +endif() +set(TEST_PYTHONPATH ${TEST_PYTHONPATH}:$ENV{PYTHONPATH}) + # Environment variables required: -set(TEST_PYTHONPATH ${CMAKE_CURRENT_SOURCE_DIR}:$ENV{PYTHONPATH}) set( PYTHON_TEST_ENVIRONMENT "SPDLOG_LEVEL=debug;PHLEX_PLUGIN_PATH=${PROJECT_BINARY_DIR};PYTHONPATH=${TEST_PYTHONPATH};PHLEX_INSTALL=${PYTHON_TEST_PHLEX_INSTALL}" @@ -136,3 +253,10 @@ set_tests_properties( ENVIRONMENT "${PYTHON_TEST_ENVIRONMENT};VIRTUAL_ENV=${PY_VIRTUAL_ENV_DIR}" ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:${PY_VIRTUAL_ENV_DIR}/bin" ) + +# Unit tests for the phlex python package +add_test( + NAME py:unit_annotations + COMMAND ${Python_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/unit_test_annotations.py +) +set_tests_properties(py:unit_annotations PROPERTIES ENVIRONMENT "${PYTHON_TEST_ENVIRONMENT}") diff --git a/test/python/adder.py b/test/python/adder.py index 549dcdab..bf496d55 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 phlex import AdjustAnnotations + + +class AddableProtocol[T](Protocol): + """Typer bound for any types that can be added.""" + + def __add__(self, other: T) -> T: # noqa: D105 + ... # codeql[py/ineffectual-statement] + + +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 = AdjustAnnotations(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/check_sys_path.py b/test/python/check_sys_path.py index f767c4dd..9d7a7acc 100644 --- a/test/python/check_sys_path.py +++ b/test/python/check_sys_path.py @@ -14,7 +14,7 @@ class Checker: site-packages directory appears in Python's sys.path. """ - __name__ = 'checker' + __name__ = "checker" def __init__(self, venv_path: str): """Initialize the checker with the expected virtual environment path. @@ -36,7 +36,8 @@ def __call__(self, i: int) -> None: """ assert len(sys.path) > 0 venv_site_packages = ( - f"{sys.prefix}/lib/python{sys.version_info.major}.{sys.version_info.minor}/site-packages" + f"{sys.prefix}/lib/python{sys.version_info.major}." + f"{sys.version_info.minor}/site-packages" ) assert any(p == venv_site_packages for p in sys.path) @@ -51,4 +52,4 @@ def PHLEX_REGISTER_ALGORITHMS(m, config): Returns: None """ - m.observe(Checker(config["venv"]), input_family = config["input"]) + m.observe(Checker(config["venv"]), input_family=config["input"]) diff --git a/test/python/failing_test_wrap.sh b/test/python/failing_test_wrap.sh deleted file mode 100755 index ee808131..00000000 --- a/test/python/failing_test_wrap.sh +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/bash -"$@" -exit_code=$? -if [ $exit_code -ne 0 ]; then - exit 1 -fi -exit 0 diff --git a/test/python/pybadbool.jsonnet b/test/python/pybadbool.jsonnet new file mode 100644 index 00000000..d1e52558 --- /dev/null +++ b/test/python/pybadbool.jsonnet @@ -0,0 +1,26 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { parent: 'job', total: 1, starting_number: 1 } + } + }, + sources: { + provider: { + cpp: 'cppsource4py', + } + }, + modules: { + test_bad_bool: { + py: 'test_callbacks', + mode: 'bad_bool', + input: ['i'], + output: ['out_bool'], + }, + verify_bool: { + py: 'verify', + input: ['out_bool'], + expected_bool: true, + } + } +} diff --git a/test/python/pybadint.jsonnet b/test/python/pybadint.jsonnet new file mode 100644 index 00000000..b3fab8fa --- /dev/null +++ b/test/python/pybadint.jsonnet @@ -0,0 +1,21 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { parent: 'job', total: 1, starting_number: 1 } + } + }, + sources: { + provider: { + cpp: 'cppsource4py', + } + }, + modules: { + test_bad_long: { + py: 'test_callbacks', + mode: 'bad_long', + input: ['i'], + output: ['out_long'], + } + } +} diff --git a/test/python/pybaduint.jsonnet b/test/python/pybaduint.jsonnet new file mode 100644 index 00000000..35711c49 --- /dev/null +++ b/test/python/pybaduint.jsonnet @@ -0,0 +1,21 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { parent: 'job', total: 1, starting_number: 1 } + } + }, + sources: { + provider: { + cpp: 'cppsource4py', + } + }, + modules: { + test_bad_uint: { + py: 'test_callbacks', + mode: 'bad_uint', + input: ['i'], + output: ['out_uint'], + } + } +} diff --git a/test/python/pycallback3.jsonnet b/test/python/pycallback3.jsonnet new file mode 100644 index 00000000..7f2a4253 --- /dev/null +++ b/test/python/pycallback3.jsonnet @@ -0,0 +1,29 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { parent: 'job', total: 1, starting_number: 1 } + } + }, + sources: { + provider: { + cpp: 'cppsource4py', + } + }, + modules: { + # 1. Test 3-arg callback (success case) + test_three_args: { + py: 'test_callbacks', + mode: 'three_args', + input: ['i', 'j', 'k'], + output: ['sum_ijk'], + }, + verify_three: { + py: 'verify', + input: ['sum_ijk'], + sum_total: 1, # 1 event * (0+0+0? wait, i=event_num-1. event1->0. sum=0. ) + # provider generates i, j starting at 0? + # cppsource4py probably uses event number. + } + } +} diff --git a/test/python/pycoverage.jsonnet b/test/python/pycoverage.jsonnet new file mode 100644 index 00000000..bd67b970 --- /dev/null +++ b/test/python/pycoverage.jsonnet @@ -0,0 +1,18 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { parent: 'job', total: 1, starting_number: 1 } + } + }, + sources: { + cppdriver: { + cpp: 'cppsource4py', + }, + }, + modules: { + coverage: { + py: 'test_coverage', + } + } +} diff --git a/test/python/pymismatch.jsonnet b/test/python/pymismatch.jsonnet new file mode 100644 index 00000000..a3b1abbf --- /dev/null +++ b/test/python/pymismatch.jsonnet @@ -0,0 +1,13 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { total: 1 } + } + }, + modules: { + mismatch: { + py: 'test_mismatch', + } + } +} diff --git a/test/python/pymismatch_annotations.jsonnet b/test/python/pymismatch_annotations.jsonnet new file mode 100644 index 00000000..f45119e9 --- /dev/null +++ b/test/python/pymismatch_annotations.jsonnet @@ -0,0 +1,22 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { parent: 'job', total: 1, starting_number: 1 } + } + }, + sources: { + provider: { + cpp: 'cppsource4py', + } + }, + modules: { + test_mismatch: { + py: 'test_callbacks', + mode: 'mismatch', + # Providing 3 inputs for a 2-arg function + input: ['i', 'j', 'k'], + output: ['sum_out'], + } + } +} diff --git a/test/python/pyraise.jsonnet b/test/python/pyraise.jsonnet new file mode 100644 index 00000000..6c6dd7e0 --- /dev/null +++ b/test/python/pyraise.jsonnet @@ -0,0 +1,21 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { parent: 'job', total: 1, starting_number: 1 } + } + }, + sources: { + provider: { + cpp: 'cppsource4py', + } + }, + modules: { + test_exception: { + py: 'test_callbacks', + mode: 'exception', + input: ['i'], + output: ['out'], + } + } +} diff --git a/test/python/pytypes.jsonnet b/test/python/pytypes.jsonnet new file mode 100644 index 00000000..27fd6a0e --- /dev/null +++ b/test/python/pytypes.jsonnet @@ -0,0 +1,33 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { parent: 'job', total: 10, starting_number: 1 } + } + }, + sources: { + cppdriver: { + cpp: 'cppsource4py', + }, + }, + modules: { + pytypes: { + py: 'test_types', + input_float: ['f1', 'f2'], + output_float: ['sum_f'], + input_double: ['d1', 'd2'], + output_double: ['sum_d'], + input_uint: ['u1', 'u2'], + output_uint: ['sum_u'], + input_bool: ['b1', 'b2'], + output_bool: ['and_b'], + output_vfloat: ['vec_f'], + output_vdouble: ['vec_d'], + }, + verify_bool: { + py: 'verify_extended', + input_bool: ['and_b'], + expected_bool: false, + }, + }, +} diff --git a/test/python/pyveclists.jsonnet b/test/python/pyveclists.jsonnet new file mode 100644 index 00000000..dfcbc3ff --- /dev/null +++ b/test/python/pyveclists.jsonnet @@ -0,0 +1,61 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { parent: "job", total: 10, starting_number: 1 } + } + }, + sources: { + cppdriver: { + cpp: 'cppsource4py', + }, + }, + modules: { + vectypes: { + py: 'vectypes', + use_lists: true, + input_int32: ['i', 'j'], + output_int32: ['sum_int32'], + input_uint32: ['u1', 'u2'], + output_uint32: ['sum_uint32'], + input_int64: ['l1', 'l2'], + output_int64: ['sum_int64'], + input_uint64: ['ul1', 'ul2'], + output_uint64: ['sum_uint64'], + input_float32: ['f1', 'f2'], + output_float32: ['sum_float32'], + input_float64: ['d1', 'd2'], + output_float64: ['sum_float64'], + }, + verify_int32: { + py: 'verify_extended', + input_int: ['sum_int32'], + sum_total: 1, + }, + verify_uint32: { + py: 'verify_extended', + input_uint: ['sum_uint32'], + sum_total: 1, + }, + verify_int64: { + py: 'verify_extended', + input_long: ['sum_int64'], + sum_total: 1, + }, + verify_uint64: { + py: 'verify_extended', + input_ulong: ['sum_uint64'], + sum_total: 100, + }, + verify_float32: { + py: 'verify_extended', + input_float: ['sum_float32'], + sum_total: 1.0, + }, + verify_double: { + py: 'verify_extended', + input_double: ['sum_float64'], + sum_total: 1.0, + }, + }, +} diff --git a/test/python/pyvectypes.jsonnet b/test/python/pyvectypes.jsonnet new file mode 100644 index 00000000..a655687b --- /dev/null +++ b/test/python/pyvectypes.jsonnet @@ -0,0 +1,60 @@ +{ + driver: { + cpp: 'generate_layers', + layers: { + event: { parent: "job", total: 10, starting_number: 1 } + } + }, + sources: { + cppdriver: { + cpp: 'cppsource4py', + }, + }, + modules: { + vectypes: { + py: 'vectypes', + input_int32: ['i', 'j'], + output_int32: ['sum_int32'], + input_uint32: ['u1', 'u2'], + output_uint32: ['sum_uint32'], + input_int64: ['l1', 'l2'], + output_int64: ['sum_int64'], + input_uint64: ['ul1', 'ul2'], + output_uint64: ['sum_uint64'], + input_float32: ['f1', 'f2'], + output_float32: ['sum_float32'], + input_float64: ['d1', 'd2'], + output_float64: ['sum_float64'], + }, + verify_int32: { + py: 'verify_extended', + input_int: ['sum_int32'], + sum_total: 1, + }, + verify_uint32: { + py: 'verify_extended', + input_uint: ['sum_uint32'], + sum_total: 1, + }, + verify_int64: { + py: 'verify_extended', + input_long: ['sum_int64'], + sum_total: 1, + }, + verify_uint64: { + py: 'verify_extended', + input_ulong: ['sum_uint64'], + sum_total: 100, + }, + verify_float32: { + py: 'verify_extended', + input_float: ['sum_float32'], + sum_total: 1.0, + }, + verify_double: { + py: 'verify_extended', + input_double: ['sum_float64'], + sum_total: 1.0, + }, + }, +} diff --git a/test/python/source.cpp b/test/python/source.cpp index 2a6aac8f..a08cb01c 100644 --- a/test/python/source.cpp +++ b/test/python/source.cpp @@ -1,12 +1,64 @@ #include "phlex/source.hpp" #include "phlex/model/data_cell_index.hpp" +#include using namespace phlex; PHLEX_REGISTER_PROVIDERS(s) { - s.provide("provide_i", [](data_cell_index const& id) -> int { return id.number(); }) + s.provide("provide_i", [](data_cell_index const& id) -> int { return id.number() % 2; }) .output_product("i"_in("job")); - s.provide("provide_j", [](data_cell_index const& id) -> int { return -id.number() + 1; }) + s.provide("provide_j", + [](data_cell_index const& id) -> int { return 1 - (int)(id.number() % 2); }) .output_product("j"_in("job")); + s.provide("provide_k", [](data_cell_index const&) -> int { return 0; }) + .output_product("k"_in("job")); + + s.provide("provide_f1", + [](data_cell_index const& id) -> float { return (float)((id.number() % 100) / 100.0); }) + .output_product("f1"_in("job")); + s.provide( + "provide_f2", + [](data_cell_index const& id) -> float { return 1.0f - (float)((id.number() % 100) / 100.0); }) + .output_product("f2"_in("job")); + + s.provide( + "provide_d1", + [](data_cell_index const& id) -> double { return (double)((id.number() % 100) / 100.0); }) + .output_product("d1"_in("job")); + s.provide("provide_d2", + [](data_cell_index const& id) -> double { + return 1.0 - (double)((id.number() % 100) / 100.0); + }) + .output_product("d2"_in("job")); + + s.provide( + "provide_u1", + [](data_cell_index const& id) -> unsigned int { return (unsigned int)(id.number() % 2); }) + .output_product("u1"_in("job")); + s.provide( + "provide_u2", + [](data_cell_index const& id) -> unsigned int { return 1 - (unsigned int)(id.number() % 2); }) + .output_product("u2"_in("job")); + + s.provide("provide_l1", [](data_cell_index const& id) -> long { return (long)(id.number() % 2); }) + .output_product("l1"_in("job")); + s.provide("provide_l2", + [](data_cell_index const& id) -> long { return 1 - (long)(id.number() % 2); }) + .output_product("l2"_in("job")); + + s.provide( + "provide_ul1", + [](data_cell_index const& id) -> unsigned long { return (unsigned long)(id.number() % 101); }) + .output_product("ul1"_in("job")); + s.provide("provide_ul2", + [](data_cell_index const& id) -> unsigned long { + return 100 - (unsigned long)(id.number() % 101); + }) + .output_product("ul2"_in("job")); + + s.provide("provide_b1", [](data_cell_index const& id) -> bool { return (id.number() % 2) == 0; }) + .output_product("b1"_in("job")); + s.provide("provide_b2", [](data_cell_index const& id) -> bool { return (id.number() % 2) != 0; }) + .output_product("b2"_in("job")); } diff --git a/test/python/test_callbacks.py b/test/python/test_callbacks.py new file mode 100644 index 00000000..b43e4193 --- /dev/null +++ b/test/python/test_callbacks.py @@ -0,0 +1,58 @@ +"""Test coverage gaps in modulewrap.cpp.""" + + +# 3-argument function to trigger py_callback<3> +def sum_three(a: int, b: int, c: int) -> int: + """Sum three integers.""" + return a + b + c + + +# Function that raises exception to test error handling +def raise_error(a: int) -> int: + """Raise a RuntimeError.""" + raise RuntimeError("Intentional failure") + + +# Invalid bool return (2) +def bad_bool(a: int) -> bool: + """Return an invalid boolean value.""" + return 2 # type: ignore + + +# Invalid long return (float) +def bad_long(a: int) -> "long": # type: ignore # noqa: F821 + """Return a float instead of an int.""" + return 1.5 # type: ignore + + +# Invalid uint return (negative) +def bad_uint(a: int) -> "unsigned int": # type: ignore # noqa: F722 + """Return a negative value for unsigned int.""" + return -5 # type: ignore + + +# Function with mismatching annotation count vs config inputs +def two_args(a: int, b: int) -> int: + """Sum two integers.""" + return a + b + + +def PHLEX_REGISTER_ALGORITHMS(m, config): + """Register algorithms based on configuration.""" + try: + mode = config["mode"] + except KeyError: + mode = "three_args" + + if mode == "three_args": + m.transform(sum_three, input_family=config["input"], output_products=config["output"]) + elif mode == "exception": + m.transform(raise_error, input_family=config["input"], output_products=config["output"]) + elif mode == "bad_bool": + m.transform(bad_bool, input_family=config["input"], output_products=config["output"]) + elif mode == "bad_long": + m.transform(bad_long, input_family=config["input"], output_products=config["output"]) + elif mode == "bad_uint": + m.transform(bad_uint, input_family=config["input"], output_products=config["output"]) + elif mode == "mismatch": + m.transform(two_args, input_family=config["input"], output_products=config["output"]) diff --git a/test/python/test_coverage.py b/test/python/test_coverage.py new file mode 100644 index 00000000..04dc3cf5 --- /dev/null +++ b/test/python/test_coverage.py @@ -0,0 +1,51 @@ +"""Test coverage for list input converters.""" + + +class double(float): # noqa: N801 + """Dummy class for C++ double type.""" + + pass + + +def list_int_func(lst: list[int]) -> int: + """Sum a list of integers.""" + return sum(lst) + + +def list_float_func(lst: list[float]) -> float: + """Sum a list of floats.""" + return sum(lst) + + +# For double, I'll use string annotation to be safe and match C++ check +def list_double_func(lst: "list[double]") -> float: # type: ignore + """Sum a list of doubles.""" + return sum(lst) + + +def collect_int(i: int) -> list[int]: + """Collect an integer into a list.""" + return [i] + + +def collect_float(f: float) -> list[float]: + """Collect a float into a list.""" + return [f] + + +def collect_double(d: "double") -> "list[double]": # type: ignore + """Collect a double into a list.""" + return [d] + + +def PHLEX_REGISTER_ALGORITHMS(m, config): + """Register algorithms.""" + # We need to transform scalar inputs to lists first + # i, f1, d1 come from cppsource4py + m.transform(collect_int, input_family=["i"], output_products=["l_int"]) + m.transform(collect_float, input_family=["f1"], output_products=["l_float"]) + m.transform(collect_double, input_family=["d1"], output_products=["l_double"]) + + m.transform(list_int_func, input_family=["l_int"], output_products=["sum_int"]) + m.transform(list_float_func, input_family=["l_float"], output_products=["sum_float"]) + m.transform(list_double_func, input_family=["l_double"], output_products=["sum_double"]) diff --git a/test/python/test_mismatch.py b/test/python/test_mismatch.py new file mode 100644 index 00000000..e8f2f769 --- /dev/null +++ b/test/python/test_mismatch.py @@ -0,0 +1,13 @@ +"""Test mismatch between input labels and types.""" + + +def mismatch_func(a: int, b: int): + """Add two integers.""" + return a + b + + +def PHLEX_REGISTER_ALGORITHMS(m, config): + """Register algorithms.""" + # input_family has 1 element, but function takes 2 arguments + # This should trigger the error in modulewrap.cpp + m.transform(mismatch_func, input_family=["a"], output_products=["sum"]) diff --git a/test/python/test_types.py b/test/python/test_types.py new file mode 100644 index 00000000..bacd3a09 --- /dev/null +++ b/test/python/test_types.py @@ -0,0 +1,127 @@ +"""Algorithms exercising various C++ types. + +This test code implements algorithms that use types other than the standard +int/string to ensure that the Python bindings correctly handle them. +""" + +import numpy as np +import numpy.typing as npt + + +class double(float): # noqa: N801 + """Dummy class for C++ double type.""" + + pass + + +def add_float(i: float, j: float) -> float: + """Add two floats. + + Args: + i (float): First input. + j (float): Second input. + + Returns: + float: Sum of the two inputs. + """ + return i + j + + +def add_double(i: double, j: double) -> double: + """Add two doubles. + + Args: + i (float): First input. + j (float): Second input. + + Returns: + float: Sum of the two inputs. + """ + return double(i + j) + + +def add_unsigned(i: "unsigned int", j: "unsigned int") -> "unsigned int": # type: ignore # noqa: F722 + """Add two unsigned integers. + + Args: + i (int): First input. + j (int): Second input. + + Returns: + int: Sum of the two inputs. + """ + return i + j + + +def collect_float(i: float, j: float) -> npt.NDArray[np.float32]: + """Combine floats into a numpy array. + + Args: + i (float): First input. + j (float): Second input. + + Returns: + ndarray: Array of floats. + """ + return np.array([i, j], dtype=np.float32) + + +def collect_double(i: double, j: double) -> npt.NDArray[np.float64]: + """Combine doubles into a numpy array. + + Args: + i (float): First input. + j (float): Second input. + + Returns: + ndarray: Array of doubles. + """ + return np.array([i, j], dtype=np.float64) + + +def and_bool(i: bool, j: bool) -> bool: + """And two booleans. + + Args: + i (bool): First input. + j (bool): Second input. + + Returns: + bool: Logical AND of the two inputs. + """ + return i and j + + +def PHLEX_REGISTER_ALGORITHMS(m, config): + """Register algorithms. + + Args: + m (internal): Phlex registrar representation. + config (internal): Phlex configuration representation. + + Returns: + None + """ + m.transform( + add_float, input_family=config["input_float"], output_products=config["output_float"] + ) + + m.transform( + add_double, input_family=config["input_double"], output_products=config["output_double"] + ) + + m.transform( + add_unsigned, input_family=config["input_uint"], output_products=config["output_uint"] + ) + + m.transform(and_bool, input_family=config["input_bool"], output_products=config["output_bool"]) + + m.transform( + collect_float, input_family=config["input_float"], output_products=config["output_vfloat"] + ) + + m.transform( + collect_double, + input_family=config["input_double"], + output_products=config["output_vdouble"], + ) diff --git a/test/python/unit_test_annotations.py b/test/python/unit_test_annotations.py new file mode 100644 index 00000000..91d09a00 --- /dev/null +++ b/test/python/unit_test_annotations.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 +"""Unit tests for the phlex.AdjustAnnotations class.""" + +import unittest + +from phlex import AdjustAnnotations + + +def example_func(a, b=1): + """Example function for testing.""" + return a + b + + +class TestAdjustAnnotations(unittest.TestCase): + """Tests for AdjustAnnotations wrapper.""" + + def test_initialization(self): + """Test proper initialization and attribute exposure.""" + ann = {"a": int, "b": int, "return": int} + wrapper = AdjustAnnotations(example_func, ann, "example_wrapper") + + self.assertEqual(wrapper.__name__, "example_wrapper") + self.assertEqual(wrapper.__annotations__, ann) + self.assertEqual(wrapper.phlex_callable, example_func) + # Check introspection attributes are exposed + self.assertEqual(wrapper.__code__, example_func.__code__) + self.assertEqual(wrapper.__defaults__, example_func.__defaults__) + + def test_call_by_default_raises(self): + """Test that calling the wrapper raises AssertionError by default.""" + wrapper = AdjustAnnotations(example_func, {}, "no_call") + with self.assertRaises(AssertionError) as cm: + wrapper(1) + self.assertIn("was called directly", str(cm.exception)) + + def test_allow_call(self): + """Test that calling is allowed when configured.""" + wrapper = AdjustAnnotations(example_func, {}, "yes_call", allow_call=True) + self.assertEqual(wrapper(10, 20), 30) + + def test_clone_shallow(self): + """Test shallow cloning behavior.""" + # For a function, copy.copy just returns the function itself usually, + # but let's test the flag logic in AdjustAnnotations + wrapper = AdjustAnnotations(example_func, {}, "clone_shallow", clone=True) + # function copy is same object + self.assertEqual(wrapper.phlex_callable, example_func) + + # Test valid copy logic with a mutable callable + class CallableObj: + def __call__(self): pass + + obj = CallableObj() + wrapper_obj = AdjustAnnotations(obj, {}, "obj_clone", clone=True) + self.assertNotEqual(id(wrapper_obj.phlex_callable), id(obj)) # copy was made? + # copy.copy of a custom object usually creates a new instance if generic + + def test_clone_deep(self): + """Test deep cloning behavior.""" + class Container: + def __init__(self): self.data = [1] + def __call__(self): return self.data[0] + + c = Container() + wrapper = AdjustAnnotations(c, {}, "deep_clone", clone="deep") + self.assertNotEqual(id(wrapper.phlex_callable), id(c)) + self.assertNotEqual(id(wrapper.phlex_callable.data), id(c.data)) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/python/vectypes.py b/test/python/vectypes.py new file mode 100644 index 00000000..db9bd342 --- /dev/null +++ b/test/python/vectypes.py @@ -0,0 +1,276 @@ +"""Algorithms exercising various numpy array types. + +This test code implements algorithms that use numpy arrays of different types +to ensure that the Python bindings correctly handle them. +""" + +import numpy as np +import numpy.typing as npt + +# Type aliases for C++ types that don't have Python equivalents +# These are used by the C++ wrapper to identify the correct converter + + +class _CppTypeMeta(type): + """Metaclass to allow overriding __name__ for C++ type identification.""" + + def __new__(mcs, name, bases, namespace, cpp_name=None): + cls = super().__new__(mcs, name, bases, namespace) + cls._cpp_name = cpp_name if cpp_name else name + return cls + + @property + def __name__(cls): + return cls._cpp_name + + +class unsigned_int(int, metaclass=_CppTypeMeta, cpp_name="unsigned int"): # noqa: N801 + """Type alias for C++ unsigned int.""" + + pass + + +class unsigned_long(int, metaclass=_CppTypeMeta, cpp_name="unsigned long"): # noqa: N801 + """Type alias for C++ unsigned long.""" + + pass + + +class long(int, metaclass=_CppTypeMeta, cpp_name="long"): # noqa: N801, A001 + """Type alias for C++ long.""" + + pass + + +class double(float, metaclass=_CppTypeMeta, cpp_name="double"): # noqa: N801 + """Type alias for C++ double.""" + + pass + + +def collectify_int32(i: int, j: int) -> npt.NDArray[np.int32]: + """Create an int32 array from two integers.""" + return np.array([i, j], dtype=np.int32) + + +def sum_array_int32(coll: npt.NDArray[np.int32] | list[int]) -> int: + """Sum an int32 array.""" + if isinstance(coll, list): + coll = np.array(coll, dtype=np.int32) + return int(sum(int(x) for x in coll)) + + +def collectify_uint32( + i: unsigned_int, + j: unsigned_int, +) -> npt.NDArray[np.uint32]: + """Create a uint32 array from two integers.""" + return np.array([i, j], dtype=np.uint32) + + +def sum_array_uint32(coll: npt.NDArray[np.uint32] | list[int]) -> unsigned_int: + """Sum a uint32 array.""" + if isinstance(coll, list): + coll = np.array(coll, dtype=np.uint32) + return unsigned_int(sum(int(x) for x in coll)) + + +def collectify_int64(i: long, j: long) -> npt.NDArray[np.int64]: + """Create an int64 array from two integers.""" + return np.array([i, j], dtype=np.int64) + + +def sum_array_int64(coll: npt.NDArray[np.int64] | list[int]) -> long: + """Sum an int64 array.""" + if isinstance(coll, list): + coll = np.array(coll, dtype=np.int64) + return long(sum(int(x) for x in coll)) + + +def collectify_uint64( + i: unsigned_long, + j: unsigned_long, +) -> npt.NDArray[np.uint64]: + """Create a uint64 array from two integers.""" + return np.array([i, j], dtype=np.uint64) + + +def sum_array_uint64(coll: npt.NDArray[np.uint64] | list[int]) -> unsigned_long: + """Sum a uint64 array.""" + if isinstance(coll, list): + coll = np.array(coll, dtype=np.uint64) + return unsigned_long(sum(int(x) for x in coll)) + + +def collectify_float32(i: float, j: float) -> npt.NDArray[np.float32]: + """Create a float32 array from two floats.""" + return np.array([i, j], dtype=np.float32) + + +def sum_array_float32(coll: npt.NDArray[np.float32]) -> float: + """Sum a float32 array.""" + return float(sum(coll)) + + +def collectify_float64(i: double, j: double) -> npt.NDArray[np.float64]: + """Create a float64 array from two floats.""" + return np.array([i, j], dtype=np.float64) + + +def collectify_float32_list(i: float, j: float) -> list[float]: + """Create a float32 list from two floats.""" + return [i, j] + + +def collectify_float64_list(i: double, j: double) -> list["double"]: + """Create a float64 list from two floats.""" + return [i, j] + + +def sum_array_float64(coll: npt.NDArray[np.float64]) -> double: + """Sum a float64 array.""" + return double(sum(coll)) + + +def collectify_int32_list(i: int, j: int) -> list[int]: + """Create an int32 list from two integers.""" + return [i, j] + + +def collectify_uint32_list( + i: unsigned_int, + j: unsigned_int, +) -> list[int]: + """Create a uint32 list from two integers.""" + return [int(i), int(j)] + + +def collectify_int64_list(i: long, j: long) -> list[int]: + """Create an int64 list from two integers.""" + return [int(i), int(j)] + + +def collectify_uint64_list( + i: unsigned_long, + j: unsigned_long, +) -> list[int]: + """Create a uint64 list from two integers.""" + return [int(i), int(j)] + + +def sum_list_int32(coll: list[int]) -> int: + """Sum a list of ints.""" + return sum(coll) + + +def sum_list_uint32(coll: list[int]) -> unsigned_int: + """Sum a list of uints.""" + return unsigned_int(sum(coll)) + + +def sum_list_int64(coll: list[int]) -> long: + """Sum a list of longs.""" + return long(sum(coll)) + + +def sum_list_uint64(coll: list[int]) -> unsigned_long: + """Sum a list of ulongs.""" + return unsigned_long(sum(coll)) + + +def sum_list_float(coll: list[float]) -> float: + """Sum a list of floats.""" + return sum(coll) + + +def sum_list_double(coll: list["double"]) -> double: + """Sum a list of doubles.""" + return double(sum(coll)) + + +def PHLEX_REGISTER_ALGORITHMS(m, config): + """Register algorithms for the test.""" + try: + use_lists = config["use_lists"] + except (KeyError, TypeError): + use_lists = False + + specs = [ + ( + "int32", + collectify_int32_list, + collectify_int32, + sum_list_int32, + sum_array_int32, + "input_int32", + "output_int32", + "sum_int32", + ), + ( + "uint32", + collectify_uint32_list, + collectify_uint32, + sum_list_uint32, + sum_array_uint32, + "input_uint32", + "output_uint32", + "sum_uint32", + ), + ( + "int64", + collectify_int64_list, + collectify_int64, + sum_list_int64, + sum_array_int64, + "input_int64", + "output_int64", + "sum_int64", + ), + ( + "uint64", + collectify_uint64_list, + collectify_uint64, + sum_list_uint64, + sum_array_uint64, + "input_uint64", + "output_uint64", + "sum_uint64", + ), + ( + "float32", + collectify_float32_list, + collectify_float32, + sum_list_float, + sum_array_float32, + "input_float32", + "output_float32", + None, + ), + ( + "float64", + collectify_float64_list, + collectify_float64, + sum_list_double, + sum_array_float64, + "input_float64", + "output_float64", + None, + ), + ] + + for name, list_collect, arr_collect, list_sum, arr_sum, in_key, out_key, sum_name in specs: + arr_name = f"arr_{name}" + m.transform( + list_collect if use_lists else arr_collect, + input_family=config[in_key], + output_products=[arr_name], + ) + + sum_kwargs = { + "input_family": [arr_name], + "output_products": config[out_key], + } + if sum_name: + sum_kwargs["name"] = sum_name + + m.transform(list_sum if use_lists else arr_sum, **sum_kwargs) diff --git a/test/python/verify.py b/test/python/verify.py index 936f5a81..5fb46c9c 100644 --- a/test/python/verify.py +++ b/test/python/verify.py @@ -54,6 +54,20 @@ def __call__(self, value: int) -> None: assert value == self._sum_total +class BoolVerifier: + """Verifier for boolean values.""" + + __name__ = "bool_verifier" + + def __init__(self, expected: bool): + """Create a boolean verifier.""" + self._expected = expected + + def __call__(self, value: bool) -> None: + """Verify the boolean value.""" + assert value == self._expected + + def PHLEX_REGISTER_ALGORITHMS(m, config): """Register an instance of `Verifier` as an observer. @@ -68,5 +82,13 @@ def PHLEX_REGISTER_ALGORITHMS(m, config): Returns: None """ + try: + expected = config["expected_bool"] + v = BoolVerifier(expected) + m.observe(v, input_family=config["input"]) + return + except Exception: + pass + assert_sum = Verifier(config["sum_total"]) m.observe(assert_sum, input_family=config["input"]) diff --git a/test/python/verify_extended.py b/test/python/verify_extended.py new file mode 100644 index 00000000..c456cde1 --- /dev/null +++ b/test/python/verify_extended.py @@ -0,0 +1,144 @@ +"""Observers to check for various types in tests.""" + + +class VerifierInt: + """Verify int values.""" + + __name__ = "verifier_int" + + def __init__(self, sum_total: int): + """Initialize with expected sum.""" + self._sum_total = sum_total + + def __call__(self, value: int) -> None: + """Check if value matches expected sum.""" + assert value == self._sum_total + + +class VerifierUInt: + """Verify unsigned int values.""" + + __name__ = "verifier_uint" + + def __init__(self, sum_total: int): + """Initialize with expected sum.""" + self._sum_total = sum_total + + def __call__(self, value: "unsigned int") -> None: # type: ignore # noqa: F722 + """Check if value matches expected sum.""" + assert value == self._sum_total + + +class VerifierLong: + """Verify long values.""" + + __name__ = "verifier_long" + + def __init__(self, sum_total: int): + """Initialize with expected sum.""" + self._sum_total = sum_total + + def __call__(self, value: "long") -> None: # type: ignore # noqa: F821 + """Check if value matches expected sum.""" + assert value == self._sum_total + + +class VerifierULong: + """Verify unsigned long values.""" + + __name__ = "verifier_ulong" + + def __init__(self, sum_total: int): + """Initialize with expected sum.""" + self._sum_total = sum_total + + def __call__(self, value: "unsigned long") -> None: # type: ignore # noqa: F722 + """Check if value matches expected sum.""" + assert value == self._sum_total + + +class VerifierFloat: + """Verify float values.""" + + __name__ = "verifier_float" + + def __init__(self, sum_total: float): + """Initialize with expected sum.""" + self._sum_total = sum_total + + def __call__(self, value: "float") -> None: + """Check if value matches expected sum.""" + assert abs(value - self._sum_total) < 1e-5 + + +class VerifierDouble: + """Verify double values.""" + + __name__ = "verifier_double" + + def __init__(self, sum_total: float): + """Initialize with expected sum.""" + self._sum_total = sum_total + + def __call__(self, value: "double") -> None: # type: ignore # noqa: F821 + """Check if value matches expected sum.""" + assert abs(value - self._sum_total) < 1e-5 + + +class VerifierBool: + """Verify bool values.""" + + __name__ = "verifier_bool" + + def __init__(self, expected: bool): + """Initialize with expected value.""" + self._expected = expected + + def __call__(self, value: bool) -> None: + """Check if value matches expected.""" + assert value == self._expected + + +def PHLEX_REGISTER_ALGORITHMS(m, config): + """Register observers for the test.""" + try: + m.observe(VerifierInt(config["sum_total"]), input_family=config["input_int"]) + except (KeyError, TypeError): + # Optional configuration, skip if missing + pass + + try: + m.observe(VerifierBool(config["expected_bool"]), input_family=config["input_bool"]) + except (KeyError, TypeError): + # Optional configuration, skip if missing + pass + + try: + m.observe(VerifierUInt(config["sum_total"]), input_family=config["input_uint"]) + except (KeyError, TypeError): + # Optional configuration, skip if missing + pass + + try: + m.observe(VerifierLong(config["sum_total"]), input_family=config["input_long"]) + except (KeyError, TypeError): + # Optional configuration, skip if missing + pass + + try: + m.observe(VerifierULong(config["sum_total"]), input_family=config["input_ulong"]) + except (KeyError, TypeError): + # Optional configuration, skip if missing + pass + + try: + m.observe(VerifierFloat(config["sum_total"]), input_family=config["input_float"]) + except (KeyError, TypeError): + # Optional configuration, skip if missing + pass + + try: + m.observe(VerifierDouble(config["sum_total"]), input_family=config["input_double"]) + except (KeyError, TypeError): + # Optional configuration, skip if missing + pass