From a77324bb76e825b63c4c03db0e2b479738219dd8 Mon Sep 17 00:00:00 2001 From: SYMSCAE Date: Sun, 11 Oct 2026 10:39:10 -0700 Subject: [PATCH 1/3] fix: fix QuantityType slice assignment --- src/extensions/wrap_PairQuantity.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/extensions/wrap_PairQuantity.cpp b/src/extensions/wrap_PairQuantity.cpp index 628ec2c..ea7c2dd 100644 --- a/src/extensions/wrap_PairQuantity.cpp +++ b/src/extensions/wrap_PairQuantity.cpp @@ -354,8 +354,11 @@ void set_quantity_slice(QuantityType& values, nb::slice slice, nb::object items) { auto [first, last, step, count] = slice.compute(values.size()); // Finish conversion before editing, including when items aliases values. - QuantityType buffer; - QuantityType replacement = extractQuantityType(items, buffer); + // Convert elements as the original vector binding did, so NumPy handles + // byte order and strides instead of using extractQuantityType's raw buffer. + QuantityType replacement; + for (nb::handle item : items) + replacement.push_back(nb::cast(item)); if (step == 1) { From ded06b12b50513d4d2e729ab7adb8e3f70cf32d7 Mon Sep 17 00:00:00 2001 From: SYMSCAE Date: Sun, 11 Oct 2026 11:17:18 -0700 Subject: [PATCH 2/3] Modernize array handling with nanobind Replace the NumPy C API with typed nanobind arrays, preserving NumPy-owned results and pinned Atom views. Correct strided and byte-swapped inputs and reject overflowing integer indices. NumPy remains a runtime dependency but is no longer needed to build. Keep focused regression and ownership checks in the existing test modules. Validation: 433 tests passed on Python 3.13; 176 targeted tests passed on each of Python 3.12 and 3.14, with two optional ObjCryst skips per environment. --- CMakeLists.txt | 7 +- news/nanobind-arrays.rst | 22 + pyproject.toml | 1 - src/extensions/srreal_atom.hpp | 7 +- src/extensions/srreal_converters.cpp | 429 ++++++++---------- src/extensions/srreal_converters.hpp | 76 ++-- src/extensions/srreal_ext.cpp | 15 - src/extensions/srreal_numpy_symbol.hpp | 29 -- .../wrap_AtomicStructureAdapter.cpp | 16 +- src/extensions/wrap_PDFBaseline.cpp | 21 +- src/extensions/wrap_PDFEnvelope.cpp | 21 +- src/extensions/wrap_PairQuantity.cpp | 8 +- src/extensions/wrap_ScatteringFactorTable.cpp | 21 +- tests/test_pairquantity.py | 66 ++- tests/test_pdfcalculator.py | 18 + tests/test_python_interfaces.py | 35 ++ tests/test_quantitytype.py | 31 ++ tests/test_structure_lifetimes.py | 22 + tests/test_structureadapter.py | 24 + 19 files changed, 483 insertions(+), 386 deletions(-) create mode 100644 news/nanobind-arrays.rst delete mode 100644 src/extensions/srreal_numpy_symbol.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 1573731..dc25fa2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,7 +11,7 @@ if(DEFINED ENV{CONDA_PREFIX}) list(APPEND CMAKE_PREFIX_PATH "$ENV{CONDA_PREFIX}" "$ENV{CONDA_PREFIX}/Library") endif() -find_package(Python 3.12 REQUIRED COMPONENTS Interpreter Development.Module NumPy) +find_package(Python 3.12 REQUIRED COMPONENTS Interpreter Development.Module) execute_process( COMMAND "${Python_EXECUTABLE}" -m nanobind --cmake_dir OUTPUT_VARIABLE nanobind_DIR @@ -52,10 +52,9 @@ target_compile_definitions(diffpy_core PRIVATE DIFFPYRUNTIMERELPATH=data) target_link_libraries(diffpy_core PUBLIC Boost::serialization GSL::gsl GSL::gslcblas ${CMAKE_DL_LIBS}) file(GLOB SRREAL_SOURCES CONFIGURE_DEPENDS "src/extensions/*.cpp") -# Keep the CPython-specific ABI while the bindings use the NumPy C API and -# CPython macros. Do not label these wheels abi3 without porting those calls. +# Retain CPython-specific wheels; stable-ABI packaging is a separate change. nanobind_add_module(srreal_ext NB_STATIC ${SRREAL_SOURCES}) -target_link_libraries(srreal_ext PRIVATE diffpy_core Python::NumPy) +target_link_libraries(srreal_ext PRIVATE diffpy_core) set_target_properties(srreal_ext PROPERTIES INSTALL_RPATH_USE_LINK_PATH ON) install(TARGETS srreal_ext LIBRARY DESTINATION diffpy/srreal) install(DIRECTORY "${LIBDIFFPY_SOURCE_DIR}/src/runtime/" diff --git a/news/nanobind-arrays.rst b/news/nanobind-arrays.rst new file mode 100644 index 0000000..1fcb8e3 --- /dev/null +++ b/news/nanobind-arrays.rst @@ -0,0 +1,22 @@ +**Changed:** + +* Use typed nanobind arrays for numeric input, copied results, and writable + Atom views. Keep NumPy output types, shapes, native dtypes, and the existing + scalar-versus-array return conventions. +* Remove the NumPy C API and build-time NumPy dependency. NumPy remains a + runtime dependency; wheel Python ABI requirements are unchanged. +* Retain NumPy-owned writable storage for copied results and explicit storage + owners for Atom views, including slices retained after structural edits. +* Preserve endpoint-specific conversion rules, NumPy scalar casting warnings, + and coordinate/matrix broadcasting. Invalid coordinate/matrix assignments + no longer partially modify an Atom. + +**Fixed:** + +* Read strided and reversed integer index arrays in logical order instead of + treating their storage as contiguous. +* Interpret byte-swapped FFT inputs and integer index arrays in their actual + byte order, and safely handle unaligned buffers. +* Reject integer-array indices outside the C++ ``int`` range with + ``OverflowError`` instead of silently wrapping. These three corrections + intentionally change erroneous behavior inherited from Boost.Python. diff --git a/pyproject.toml b/pyproject.toml index d1a4286..db67cf9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -2,7 +2,6 @@ requires = [ "scikit-build-core>=0.10", "setuptools-scm>=8", - "numpy", "nanobind>=3.1,<4", ] build-backend = "scikit_build_core.build" diff --git a/src/extensions/srreal_atom.hpp b/src/extensions/srreal_atom.hpp index 4bd2460..840da21 100644 --- a/src/extensions/srreal_atom.hpp +++ b/src/extensions/srreal_atom.hpp @@ -135,16 +135,17 @@ struct AtomViewPin } }; -inline nb::object pin_atom_view(PythonAtom& atom, nb::object view) +inline nb::object atom_view_owner(PythonAtom& atom) { + nb::object python_atom = nb::cast(&atom, nb::rv_policy::reference); if (atom.references()) { nb::capsule owner(new AtomViewPin(atom.references()), [](void* p) noexcept { delete static_cast(p); }); - setNumPyArrayBase(view, std::move(owner)); + return nb::make_tuple(python_atom, owner); } - return view; + return python_atom; } inline void preserve_atom_views(const std::shared_ptr& refs) diff --git a/src/extensions/srreal_converters.cpp b/src/extensions/srreal_converters.cpp index 9dfec74..d0dd48d 100644 --- a/src/extensions/srreal_converters.cpp +++ b/src/extensions/srreal_converters.cpp @@ -13,18 +13,18 @@ ****************************************************************************** * * utilities for conversion between C++ and Python types. -* boost python converters are prone to conflicts. * *****************************************************************************/ -// TODO: replace Py_DECREF with nb::steal - #include #include #include +#include #include #include #include +#include +#include #include #include @@ -32,14 +32,6 @@ #include "srreal_converters.hpp" #include "srreal_validators.hpp" -#include "srreal_numpy_symbol.hpp" -// numpy/arrayobject.h needs to be included after srreal_numpy_symbol.hpp, -// which defines PY_ARRAY_UNIQUE_SYMBOL. NO_IMPORT_ARRAY indicates -// import_array will be called in the extension module initializer. -#define NO_IMPORT_ARRAY -#include - - namespace { // exception translations ---------------------------------------------------- @@ -47,38 +39,74 @@ namespace { using diffpy::attributes::DoubleAttributeError; using std::invalid_argument; -void translate_DoubleAttributeError(const DoubleAttributeError& e) +template +bool native_array(nb::handle obj, Array& result) { - PyErr_SetString(PyExc_AttributeError, e.what()); + // Keep scalars on their conversion path. NumPy datetime/timedelta scalar + // buffers omit strides, which nanobind's buffer importer dereferences. + if (!PySequence_Check(obj.ptr())) return false; + // The importer checks byte order, dtype and layout, but not alignment. + return nb::try_cast(obj, result, false) && + reinterpret_cast(result.data()) % + alignof(typename Array::Scalar) == 0; } +using IndexArray = nb::ndarray, + nb::device::cpu>; -void translate_invalid_argument(const invalid_argument& e) +template +bool copy_index_array(const IndexArray& array, std::vector& result) { - PyErr_SetString(PyExc_ValueError, e.what()); -} + if (array.dtype() != nb::dtype() || + reinterpret_cast(array.data()) % alignof(T) != 0) + return false; + result.resize(array.size()); + if constexpr (std::is_same_v) + { + if (array.stride(0) == 1) + { + if (!result.empty()) + std::copy_n(static_cast(array.data()), + result.size(), result.begin()); + return true; + } + } + // Typed views also handle slices without boxing each integer. + auto values = array.view(); + for (size_t i = 0; i < result.size(); ++i) + { + T value = values(i); + if (!std::in_range(value)) + { + PyErr_SetString(PyExc_OverflowError, "integer index out of range"); + nb::raise_python_error(); + } + result[i] = static_cast(value); + } + return true; +} -nb::object newNumPyArray(int dim, const int* sz, int typenum) +nb::object newNumPyArray(size_t dim, const size_t* shape, + const char* dtype, size_t itemsize) { - std::vector dims(static_cast(dim)); - std::transform( - sz, - sz + dim, - dims.begin(), - [](int v) { return static_cast(v); } - ); - - PyObject *arr = PyArray_SimpleNew( - dim, - dims.empty() ? nullptr : dims.data(), - typenum - ); - - if (!arr) - throw nb::python_error(); - - return nb::steal(arr); + // NumPy dimensions and byte counts must fit in Py_ssize_t, even though + // ndarray metadata uses size_t. Check before multiplying or allocating. + const size_t limit = static_cast(PY_SSIZE_T_MAX); + size_t count = 1; + nb::tuple_builder dimensions(dim); + for (size_t i = 0; i < dim; ++i) + { + if (shape[i] > limit || (count && shape[i] > limit / itemsize / count)) + { + PyErr_SetString(PyExc_OverflowError, "array size is out of range"); + nb::raise_python_error(); + } + count *= shape[i]; + dimensions.put(shape[i]); + } + return nb::module_::import_("numpy").attr("empty")( + dimensions.commit(), nb::arg("dtype") = dtype); } } // namespace @@ -116,167 +144,91 @@ void wrap_exceptions() } -/// helper for creating numpy array of doubles -NumPyArray_DoublePtr createNumPyDoubleArray(int dim, const int* sz) +NumPyOutput createNumPyDoubleArray(size_t dim, const size_t* sz) { - nb::object rvobj = newNumPyArray(dim, sz, NPY_DOUBLE); - PyArrayObject* a = reinterpret_cast(rvobj.ptr()); - double* rvdata = static_cast(PyArray_DATA(a)); - NumPyArray_DoublePtr rv(rvobj, rvdata); - return rv; + nb::object array = newNumPyArray(dim, sz, "float64", sizeof(double)); + return {array, nb::cast::array)>(array, false)}; } -/// helper for creating numpy array of the same shape as the argument -NumPyArray_DoublePtr createNumPyDoubleArrayLike(nb::object& obj) +NumPyOutput createNumPyDoubleArrayLike(const NumPyDoubleInput& array) { - assert(PyArray_Check(obj.ptr())); - PyArrayObject* a = reinterpret_cast(obj.ptr()); - // create numpy array - PyObject *pobj = PyArray_NewLikeArray( - a, - NPY_CORDER, - PyArray_DescrFromType(NPY_DOUBLE), - 0 - ); - - if (!pobj) - throw nb::python_error(); - - nb::object rvobj = nb::steal(pobj); - PyArrayObject* a1 = reinterpret_cast(rvobj.ptr()); - double* rvdata = static_cast(PyArray_DATA(a1)); - NumPyArray_DoublePtr rv(rvobj, rvdata); - return rv; -} - - -/// helper for creating a numpy array view on a double array -nb::object -createNumPyDoubleView(double* data, int dim, const int* sz) -{ - std::vector dims(static_cast(dim)); - std::transform( - sz, - sz + dim, - dims.begin(), - [](int v) { return static_cast(v); } - ); - - PyObject* pobj = PyArray_SimpleNewFromData( - dim, - dims.data(), - NPY_DOUBLE, - data - ); - - if (!pobj) - throw nb::python_error(); - - return nb::steal(pobj); + std::vector shape(array.ndim()); + for (size_t i = 0; i < shape.size(); ++i) shape[i] = array.shape(i); + return createNumPyDoubleArray(shape.size(), shape.data()); } -void setNumPyArrayBase(nb::handle array, nb::object owner) +NumPyOutput createNumPyIntArray(size_t dim, const size_t* sz) { - auto* a = reinterpret_cast(array.ptr()); - if (PyArray_SetBaseObject(a, owner.release().ptr()) < 0) - throw nb::python_error(); + nb::object array = newNumPyArray(dim, sz, "intc", sizeof(int)); + return {array, nb::cast::array)>(array, false)}; } -/// NumPy array view specializations for R3::Vector -nb::object viewAsNumPyArray(::diffpy::srreal::R3::Vector& v) +nb::object viewAsNumPyArray(::diffpy::srreal::R3::Vector& v, nb::handle owner) { - using namespace diffpy::srreal; - double* data = &(v[0]); - int sz = R3::Ndim; - return createNumPyDoubleView(data, 1, &sz); + using View = nb::ndarray, + nb::c_contig, nb::device::cpu>; + return View(&v[0], {3}, owner).cast(nb::rv_policy::reference); } -/// NumPy array view specializations for R3::Matrix -nb::object viewAsNumPyArray(::diffpy::srreal::R3::Matrix& mx) +nb::object viewAsNumPyArray(::diffpy::srreal::R3::Matrix& mx, nb::handle owner) { - using namespace diffpy::srreal; - double* data = &(mx(0, 0)); - int sz[2] = {R3::Ndim, R3::Ndim}; - return createNumPyDoubleView(data, 2, sz); + using View = nb::ndarray, + nb::c_contig, nb::device::cpu>; + return View(&mx(0, 0), {3, 3}, owner).cast(nb::rv_policy::reference); } -/// Copy NumPy array to R3::Vector -void assignR3Vector( - ::diffpy::srreal::R3::Vector& dst, nb::object& value) +void assignR3Vector(::diffpy::srreal::R3::Vector& dst, nb::object& value) { - using diffpy::srreal::R3::Ndim; - // If value is numpy array, try direct data access - if (PyArray_Check(value.ptr())) + using VectorInput = nb::ndarray, + nb::c_contig, nb::device::cpu>; + VectorInput source; + if (native_array(value, source)) { - PyArrayObject* a = reinterpret_cast( - PyArray_ContiguousFromAny(value.ptr(), NPY_DOUBLE, 1, 1)); - if (a && Ndim == PyArray_DIM(a, 0)) - { - double* p = static_cast(PyArray_DATA(a)); - std::ranges::copy(p, p + Ndim, dst.data().begin()); - Py_DECREF(a); - return; - } - Py_XDECREF(a); + std::copy_n(source.data(), 3, dst.data().begin()); + return; } - // handle scalar assignment double scalar; if (nb::try_cast(value, scalar)) { std::ranges::fill(dst.data().begin(), dst.data().end(), scalar); return; } - // finally assign using array view - nb::object dstview = viewAsNumPyArray(dst); - dstview[nb::slice(nb::none(), nb::none(), nb::none())] = value; + // Assign into temporary NumPy storage to preserve broadcasting, dtype + // conversion and warnings without partially updating the atom on failure. + const size_t shape[] = {3}; + auto temporary = createNumPyDoubleArray(1, shape); + temporary.object[nb::slice(nb::none(), nb::none(), nb::none())] = value; + source = nb::cast(temporary.object, false); + std::copy_n(source.data(), 3, dst.data().begin()); } -/// Copy possible NumPy array to R3::Matrix -void assignR3Matrix( - ::diffpy::srreal::R3::Matrix& dst, nb::object& value) +void assignR3Matrix(::diffpy::srreal::R3::Matrix& dst, nb::object& value) { - using diffpy::srreal::R3::Ndim; - // If value is numpy array, try direct data access - if (PyArray_Check(value.ptr())) + using MatrixInput = nb::ndarray, + nb::c_contig, nb::device::cpu>; + MatrixInput source; + if (native_array(value, source)) { - PyArrayObject* a = reinterpret_cast( - PyArray_ContiguousFromAny(value.ptr(), NPY_DOUBLE, 2, 2)); - if (a && Ndim == PyArray_DIM(a, 0) && Ndim == PyArray_DIM(a, 1)) - { - double* p = static_cast(PyArray_DATA(a)); - std::ranges::copy(p, p + Ndim * Ndim, dst.data().begin()); - Py_DECREF(a); - return; - } - Py_XDECREF(a); + std::copy_n(source.data(), 9, dst.data().begin()); + return; } - // handle scalar assignment double scalar; if (nb::try_cast(value, scalar)) { std::ranges::fill(dst.data().begin(), dst.data().end(), scalar); return; } - // finally assign using array view - nb::object dstview = viewAsNumPyArray(dst); - dstview[nb::slice(nb::none(), nb::none(), nb::none())] = value; -} - - -/// helper for creating numpy array of integers -NumPyArray_IntPtr createNumPyIntArray(int dim, const int* sz) -{ - nb::object rvobj = newNumPyArray(dim, sz, NPY_INT); - PyArrayObject* a = reinterpret_cast(rvobj.ptr()); - int* rvdata = static_cast(PyArray_DATA(a)); - NumPyArray_IntPtr rv(rvobj, rvdata); - return rv; + const size_t shape[] = {3, 3}; + auto temporary = createNumPyDoubleArray(2, shape); + temporary.object[nb::slice(nb::none(), nb::none(), nb::none())] = value; + source = nb::cast(temporary.object, false); + std::copy_n(source.data(), 9, dst.data().begin()); } @@ -291,38 +243,35 @@ extractQuantityType( QuantityType* qt = nullptr; if (nb::try_cast(obj, qt) && qt) return *qt; - // copy data directly if it is a numpy array of doubles - PyArrayObject* a = PyArray_Check(obj.ptr()) ? - reinterpret_cast(obj.ptr()) : NULL; - bool isdoublenumpyarray = a && - (1 == PyArray_NDIM(a)) && - (NPY_DOUBLE == PyArray_TYPE(a)); - if (isdoublenumpyarray) + // Normalize layout before typed access, including byte order and alignment. + // Other dtypes retain the elementwise conversion used by this endpoint. + using QuantityInput = nb::ndarray, + nb::c_contig, nb::device::cpu>; + QuantityInput direct; + if (native_array(obj, direct)) { - double* src = static_cast(PyArray_DATA(a)); - npy_intp stride = PyArray_STRIDE(a, 0) / PyArray_ITEMSIZE(a); - rv.resize(PyArray_SIZE(a)); - QuantityType::iterator dst = rv.begin(); - for (; dst != rv.end(); ++dst, src += stride) *dst = *src; + rv.resize(direct.size()); + if (direct.size()) std::copy_n(direct.data(), direct.size(), rv.begin()); return rv; } - // otherwise copy elementwise converting each element to a double - std::vector tmp; - - PyObject* iter = PyObject_GetIter(obj.ptr()); - if (!iter) - throw nb::python_error(); - - nb::object it = nb::steal(iter); - - while (PyObject* item = PyIter_Next(it.ptr())) + nb::module_ numpy = nb::module_::import_("numpy"); + if (nb::isinstance(obj, numpy.attr("ndarray")) && + nb::cast(obj.attr("ndim")) == 1 && + nb::cast(obj.attr("dtype").attr("kind")) == "f" && + nb::cast(obj.attr("itemsize")) == sizeof(double)) { - nb::object item_obj = nb::steal(item); - tmp.push_back(nb::cast(item_obj)); + nb::object normalized = numpy.attr("require")(obj, + nb::arg("dtype") = "=f8", + nb::arg("requirements") = nb::make_tuple("C", "A")); + auto array = nb::cast(normalized, false); + rv.resize(array.size()); + if (array.size()) std::copy_n(array.data(), array.size(), rv.begin()); + return rv; } - - if (PyErr_Occurred()) - throw nb::python_error(); + // otherwise copy elementwise converting each element to a double + std::vector tmp; + for (nb::handle item : obj) + tmp.push_back(nb::cast(item)); rv.assign(tmp.begin(), tmp.end()); return rv; @@ -330,19 +279,45 @@ extractQuantityType( /// efficient conversion of Python object to a numpy array of doubles -NumPyArray_DoublePtr extractNumPyDoubleArray(::nb::object& obj) +NumPyDoubleInput extractNumPyDoubleArray(nb::object obj) { - PyObject* pobj = PyArray_ContiguousFromAny(obj.ptr(), NPY_DOUBLE, 0, 0); - if (!pobj) + NumPyDoubleInput direct; + if (native_array(obj, direct)) return direct; + nb::module_ numpy = nb::module_::import_("numpy"); + try + { + // NumPy's old FromAny path used safe casting for existing arrays, + // but constructed Python sequences directly in the requested dtype. + nb::object array; + if (nb::isinstance(obj, numpy.attr("generic"))) + { + // NumPy scalars historically use scalar casting, which permits + // e.g. longdouble and complex-to-real with a ComplexWarning. + array = numpy.attr("asarray")(obj).attr("astype")("float64"); + } + else if (nb::isinstance(obj)) + { + array = numpy.attr("asarray")(obj, nb::arg("dtype") = "float64"); + } + else if (nb::isinstance(obj, numpy.attr("ndarray")) || + nb::hasattr(obj, "__array__") || PyObject_CheckBuffer(obj.ptr())) + { + array = numpy.attr("asarray")(obj).attr("astype")("float64", + nb::arg("casting") = "safe", nb::arg("copy") = false); + } + else + { + array = numpy.attr("asarray")(obj, nb::arg("dtype") = "float64"); + } + array = numpy.attr("require")(array, + nb::arg("dtype") = "=f8", + nb::arg("requirements") = nb::make_tuple("C", "A")); + return nb::cast(array, false); + } + catch (const nb::python_error&) { - PyErr_Clear(); throw nb::type_error("Cannot convert this object to numpy array."); } - nb::object rvobj = nb::steal(pobj); - PyArrayObject* a = reinterpret_cast(pobj); - double* rvdata = static_cast(PyArray_DATA(a)); - NumPyArray_DoublePtr rv(rvobj, rvdata); - return rv; } @@ -353,20 +328,16 @@ double extractdouble(nb::object obj) if (nb::try_cast(obj, x)) return x; - PyObject* pobj = obj.ptr(); - if (PyArray_CheckScalar(pobj)) + nb::module_ numpy = nb::module_::import_("numpy"); + if (nb::isinstance(obj, numpy.attr("generic")) || + (nb::isinstance(obj, numpy.attr("ndarray")) && + nb::cast(obj.attr("ndim")) == 0)) { - PyArray_Descr* descr = PyArray_DescrFromType(NPY_DOUBLE); - if (!descr) - throw nb::python_error(); - - int ok = PyArray_CastScalarToCtype(pobj, &x, descr); - Py_DECREF(descr); - - if (ok < 0) - throw nb::python_error(); - - return x; + // Preserve NumPy scalar casting (including its ComplexWarning), + // without accepting Python strings as numeric arguments. + nb::object scalar = numpy.attr("asarray")(obj) + .attr("astype")("float64").attr("item")(); + return nb::cast(scalar); } // nothing worked, call default behavior which will raise an exception return nb::cast(obj); @@ -377,14 +348,6 @@ double extractdouble(nb::object obj) int extractint(nb::object obj) { PyObject* pobj = obj.ptr(); - if (PyArray_CheckScalar(pobj)) - { - int rv = PyArray_PyIntAsInt(pobj); - if (rv == -1 && PyErr_Occurred()) - throw nb::python_error(); - return rv; - } - PyObject* idx = PyNumber_Index(pobj); if (!idx) throw nb::python_error(); @@ -409,35 +372,25 @@ int extractint(nb::object obj) std::vector extractintvector(nb::object obj) { std::vector rv; + IndexArray array; + // Import once, then dispatch on the native dtype without boxing elements. + // As in native_array(), leave scalar buffers to the scalar converter. + if (PySequence_Check(obj.ptr()) && + nb::try_cast(obj, array, false) && + (copy_index_array(array, rv) || + copy_index_array(array, rv) || + copy_index_array(array, rv) || + copy_index_array(array, rv) || + copy_index_array(array, rv) || + copy_index_array(array, rv) || + copy_index_array(array, rv) || + copy_index_array(array, rv))) + return rv; // iterable of integers if (isiterable(obj)) { - PyArrayObject* a = PyArray_Check(obj.ptr()) ? - reinterpret_cast(obj.ptr()) : NULL; - // handle numpy array of integers - bool isintegernumpyarray = - a && (1 == PyArray_NDIM(a)) && PyArray_ISINTEGER(a); - if (isintegernumpyarray) - { - nb::object aobj = obj; - if (NPY_INT != PyArray_TYPE(a)) - { - PyObject* casted = PyArray_Cast(a, NPY_INT); - if (!casted) - throw nb::python_error(); - - aobj = nb::steal(casted); - } - PyArrayObject* a1 = reinterpret_cast(aobj.ptr()); - assert(NPY_INT == PyArray_TYPE(a1)); - int* pfirst = static_cast(PyArray_DATA(a1)); - int* plast = pfirst + PyArray_SIZE(a1); - rv.assign(pfirst, plast); - return rv; - } - // otherwise translate every item in the iterable - rv.reserve(len(obj)); - + // Never narrow an array before checking each integer. Iterating also + // handles non-native byte order and unaligned data. for (nb::handle item : obj) rv.push_back(extractint(nb::borrow(item))); diff --git a/src/extensions/srreal_converters.hpp b/src/extensions/srreal_converters.hpp index 84bf03b..cdf261b 100644 --- a/src/extensions/srreal_converters.hpp +++ b/src/extensions/srreal_converters.hpp @@ -13,17 +13,14 @@ ****************************************************************************** * * utilities for conversion between C++ and Python types. -* boost python converters are prone to conflicts. * *****************************************************************************/ -// TODO: Go through type casters, some of which are found redundant or could be -// replaced with nanobind builtins. - #ifndef SRREAL_CONVERTERS_HPP_INCLUDED #define SRREAL_CONVERTERS_HPP_INCLUDED #include +#include #include #include @@ -57,6 +54,9 @@ namespace srrealmodule { using diffpy::srreal::createStructureAdapter; +using NumPyDoubleInput = nb::ndarray; + /// this macro creates a setter for overloaded method that can accept /// either instance or a type string #define DECLARE_BYTYPE_SETTER_WRAPPER(method, wrapper) \ @@ -237,23 +237,22 @@ void fillPyListWithSets(nb::list lst, const T& value) } -/// Type for numpy array object and a raw pointer to its double data -typedef std::pair NumPyArray_DoublePtr; +// Keep the original NumPy allocation so returned copies retain OWNDATA. +// The typed handle validates storage and keeps it alive during C++ access. +template +struct NumPyOutput +{ + nb::object object; + nb::ndarray array; +}; /// helper for creating numpy array of doubles -NumPyArray_DoublePtr createNumPyDoubleArray(int dim, const int* sz); +NumPyOutput createNumPyDoubleArray(size_t dim, const size_t* sz); /// helper for creating numpy array of the same shape as the argument -NumPyArray_DoublePtr createNumPyDoubleArrayLike(nb::object& obj); - - -/// helper for creating numpy views on existing double array -nb::object createNumPyDoubleView(double*, int dim, const int* sz); - -/// Retain the storage owner for an array view and any slices made from it. -void setNumPyArrayBase(nb::handle array, nb::object owner); +NumPyOutput createNumPyDoubleArrayLike(const NumPyDoubleInput& array); /// template function for converting iterables to numpy array of doubles @@ -261,10 +260,10 @@ template nb::object convertToNumPyArray(Iter first, Iter last) { - int sz = last - first; - NumPyArray_DoublePtr ap = createNumPyDoubleArray(1, &sz); - std::copy(first, last, ap.second); - return ap.first; + size_t sz = static_cast(last - first); + NumPyOutput ap = createNumPyDoubleArray(1, &sz); + std::copy(first, last, ap.array.data()); + return ap.object; } @@ -281,13 +280,13 @@ inline nb::object convertToNumPyArray(const ::diffpy::srreal::R3::Matrix& mx) { using namespace diffpy::srreal; - int sz[2] = {R3::Ndim, R3::Ndim}; - NumPyArray_DoublePtr ap = createNumPyDoubleArray(2, sz); - double* xo = ap.second; + size_t sz[2] = {R3::Ndim, R3::Ndim}; + NumPyOutput ap = createNumPyDoubleArray(2, sz); + double* xo = ap.array.data(); *(xo++) = mx(0, 0); *(xo++) = mx(0, 1); *(xo++) = mx(0, 2); *(xo++) = mx(1, 0); *(xo++) = mx(1, 1); *(xo++) = mx(1, 2); *(xo++) = mx(2, 0); *(xo++) = mx(2, 1); *(xo++) = mx(2, 2); - return ap.first; + return ap.object; } @@ -296,10 +295,9 @@ inline nb::object convertToNumPyArray(const ::std::vector& vr3v) { using namespace diffpy::srreal; - int n = vr3v.size(); - int sz[2] = {n, R3::Ndim}; - NumPyArray_DoublePtr ap = createNumPyDoubleArray(2, sz); - double* p = ap.second; + size_t sz[2] = {vr3v.size(), R3::Ndim}; + NumPyOutput ap = createNumPyDoubleArray(2, sz); + double* p = ap.array.data(); std::vector::const_iterator v = vr3v.begin(); for (; v != vr3v.end(); ++v) { @@ -307,8 +305,8 @@ convertToNumPyArray(const ::std::vector& vr3v) const double* pvlast = pv + R3::Ndim; for (; pv != pvlast; ++p, ++pv) *p = *pv; } - assert(p == ap.second + sz[0] * sz[1]); - return ap.first; + assert(p == ap.array.data() + sz[0] * sz[1]); + return ap.object; } @@ -322,12 +320,12 @@ convertToNumPyArray(const ::diffpy::srreal::QuantityType& value) /// NumPy array view specializations for R3::Vector nb::object -viewAsNumPyArray(::diffpy::srreal::R3::Vector&); +viewAsNumPyArray(::diffpy::srreal::R3::Vector&, nb::handle owner); /// NumPy array view specializations for R3::Matrix nb::object -viewAsNumPyArray(::diffpy::srreal::R3::Matrix&); +viewAsNumPyArray(::diffpy::srreal::R3::Matrix&, nb::handle owner); /// Copy possible NumPy array to R3::Vector @@ -340,22 +338,18 @@ void assignR3Matrix( ::diffpy::srreal::R3::Matrix& dst, nb::object& value); -/// Type for numpy array object and a raw pointer to its double data -typedef std::pair NumPyArray_IntPtr; - - /// helper for creating numpy array of integers -NumPyArray_IntPtr createNumPyIntArray(int dim, const int* sz); +NumPyOutput createNumPyIntArray(size_t dim, const size_t* sz); /// specialization for a vector of integers inline nb::object convertToNumPyArray(const ::std::vector& value) { - int sz = value.size(); - NumPyArray_IntPtr ap = createNumPyIntArray(1, &sz); - std::copy(value.begin(), value.end(), ap.second); - return ap.first; + size_t sz = value.size(); + NumPyOutput ap = createNumPyIntArray(1, &sz); + std::copy(value.begin(), value.end(), ap.array.data()); + return ap.object; } @@ -392,7 +386,7 @@ extractQuantityType(nb::object obj, /// efficient conversion of Python object to a numpy array of doubles -NumPyArray_DoublePtr extractNumPyDoubleArray(nb::object& obj); +NumPyDoubleInput extractNumPyDoubleArray(nb::object obj); /// extract double with a support for numpy numeric types diff --git a/src/extensions/srreal_ext.cpp b/src/extensions/srreal_ext.cpp index 5328102..1c8282a 100644 --- a/src/extensions/srreal_ext.cpp +++ b/src/extensions/srreal_ext.cpp @@ -18,9 +18,6 @@ #include -#include "srreal_numpy_symbol.hpp" -#include - // Declaration of the external wrappers -------------------------------------- namespace nb = nanobind; @@ -51,23 +48,11 @@ void wrap_OverlapCalculator(nb::module_& m); } // namespace srrealmodule -namespace { - -#if PY_MAJOR_VERSION >= 3 - void* initialize_numpy() { import_array(); return NULL; } -#else - void initialize_numpy() { import_array(); } -#endif - -} // namespace - // Module Definitions -------------------------------------------------------- NB_MODULE(srreal_ext, m) { using namespace srrealmodule; - // initialize numpy module - initialize_numpy(); // execute external wrappers wrap_libdiffpy_version(m); wrap_exceptions(); diff --git a/src/extensions/srreal_numpy_symbol.hpp b/src/extensions/srreal_numpy_symbol.hpp deleted file mode 100644 index c31cf47..0000000 --- a/src/extensions/srreal_numpy_symbol.hpp +++ /dev/null @@ -1,29 +0,0 @@ -/***************************************************************************** -* -* diffpy.srreal by DANSE Diffraction group -* Simon J. L. Billinge -* (c) 2010 The Trustees of Columbia University -* in the City of New York. All rights reserved. -* -* File coded by: Pavol Juhas -* -* See AUTHORS.txt for a list of people who contributed. -* See LICENSE_DANSE.txt for license information. -* -****************************************************************************** -* -* Definition of the PY_ARRAY_UNIQUE_SYMBOL for the srreal extension module. -* -*****************************************************************************/ - -#ifndef SRREAL_NUMPY_SYMBOL_HPP_INCLUDED -#define SRREAL_NUMPY_SYMBOL_HPP_INCLUDED - -// Specify the version of NumPy API that will be used. -#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION - -// This macro is required for extension modules that are in several files. -// It must be defined before inclusion of numpy/arrayobject.h -#define PY_ARRAY_UNIQUE_SYMBOL DIFFPY_SRREAL_NUMPY_ARRAY_SYMBOL - -#endif // SRREAL_NUMPY_SYMBOL_HPP_INCLUDED diff --git a/src/extensions/wrap_AtomicStructureAdapter.cpp b/src/extensions/wrap_AtomicStructureAdapter.cpp index 35f1416..cf42e1d 100644 --- a/src/extensions/wrap_AtomicStructureAdapter.cpp +++ b/src/extensions/wrap_AtomicStructureAdapter.cpp @@ -253,7 +253,7 @@ implicitly called from createBondGenerator.\n\ nb::object get_xyz_cartn(PythonAtom& a) { - return pin_atom_view(a, viewAsNumPyArray(a.value().xyz_cartn)); + return viewAsNumPyArray(a.value().xyz_cartn, atom_view_owner(a)); } void set_xyz_cartn(PythonAtom& a, nb::object value) @@ -264,7 +264,7 @@ void set_xyz_cartn(PythonAtom& a, nb::object value) nb::object get_uij_cartn(PythonAtom& a) { - return pin_atom_view(a, viewAsNumPyArray(a.value().uij_cartn)); + return viewAsNumPyArray(a.value().uij_cartn, atom_view_owner(a)); } void set_uij_cartn(PythonAtom& a, nb::object& value) @@ -767,11 +767,9 @@ void wrap_AtomicStructureAdapter(nb::module_& m) }, nb::is_operator()) .def("__hash__", [](const PythonAtom& a) { return hash_value(a.value()); }) .def("_get_xyz_cartn", - get_xyz_cartn, - nb::keep_alive<0,1>()) + get_xyz_cartn) .def("_get_uij_cartn", - get_uij_cartn, - nb::keep_alive<0,1>()) + get_uij_cartn) ; // now we can finalize the Atom class interface atom_class @@ -780,8 +778,7 @@ void wrap_AtomicStructureAdapter(nb::module_& m) [](PythonAtom& a, const std::string& value) { a.value().atomtype = value; }) .def_prop_rw("xyz_cartn", get_xyz_cartn, - set_xyz_cartn, - nb::keep_alive<0,1>()) + set_xyz_cartn) .def_prop_rw("xc", get_xyz<0>, set_xyz<0>, doc_Atom_xic) .def_prop_rw("yc", get_xyz<1>, set_xyz<1>, doc_Atom_xic) .def_prop_rw("zc", get_xyz<2>, set_xyz<2>, doc_Atom_xic) @@ -791,8 +788,7 @@ void wrap_AtomicStructureAdapter(nb::module_& m) doc_Atom_anisotropy) .def_prop_rw("uij_cartn", get_uij_cartn, - set_uij_cartn, - nb::keep_alive<0,1>()) + set_uij_cartn) .def_prop_rw("uc11", get_uc<0, 0>, set_uc<0, 0>, doc_Atom_uijc) .def_prop_rw("uc22", get_uc<1, 1>, set_uc<1, 1>, doc_Atom_uijc) .def_prop_rw("uc33", get_uc<2, 2>, set_uc<2, 2>, doc_Atom_uijc) diff --git a/src/extensions/wrap_PDFBaseline.cpp b/src/extensions/wrap_PDFBaseline.cpp index c10da11..b4baff3 100644 --- a/src/extensions/wrap_PDFBaseline.cpp +++ b/src/extensions/wrap_PDFBaseline.cpp @@ -24,12 +24,7 @@ #include #include -#include "srreal_numpy_symbol.hpp" -// numpy/arrayobject.h needs to be included after srreal_numpy_symbol.hpp, -// which defines PY_ARRAY_UNIQUE_SYMBOL. NO_IMPORT_ARRAY indicates -// import_array will be called in the extension module initializer. -#define NO_IMPORT_ARRAY -#include + #include "srreal_converters.hpp" #include "srreal_pickling.hpp" @@ -145,15 +140,15 @@ class PDFBaselineWrap : nb::object callnparray(const PDFBaseline* obj, nb::object& x) { // Return a float for Python scalars and an array for array-like inputs. - if (PyFloat_Check(x.ptr()) || PyLong_Check(x.ptr())) + if (nb::isinstance(x) || nb::isinstance(x)) return nb::float_((*obj)(extractdouble(x))); - NumPyArray_DoublePtr xx = extractNumPyDoubleArray(x); - NumPyArray_DoublePtr yy = createNumPyDoubleArrayLike(xx.first); - double* src = xx.second; - double* last = xx.second + PyArray_Size(xx.first.ptr()); - double* dst = yy.second; + NumPyDoubleInput xx = extractNumPyDoubleArray(x); + auto yy = createNumPyDoubleArrayLike(xx); + const double* src = xx.data(); + const double* last = src + xx.size(); + double* dst = yy.array.data(); for (; src != last; ++src, ++dst) *dst = (*obj)(*src); - return yy.first; + return yy.object; } } // namespace nswrap_PDFBaseline diff --git a/src/extensions/wrap_PDFEnvelope.cpp b/src/extensions/wrap_PDFEnvelope.cpp index 89dd7dd..e5cb712 100644 --- a/src/extensions/wrap_PDFEnvelope.cpp +++ b/src/extensions/wrap_PDFEnvelope.cpp @@ -26,12 +26,7 @@ #include #include -#include "srreal_numpy_symbol.hpp" -// numpy/arrayobject.h needs to be included after srreal_numpy_symbol.hpp, -// which defines PY_ARRAY_UNIQUE_SYMBOL. NO_IMPORT_ARRAY indicates -// import_array will be called in the extension module initializer. -#define NO_IMPORT_ARRAY -#include + #include "srreal_converters.hpp" #include "srreal_pickling.hpp" @@ -176,15 +171,15 @@ class PDFEnvelopeWrap : nb::object callnparray(const PDFEnvelope* obj, nb::object& x) { - if (PyFloat_Check(x.ptr()) || PyLong_Check(x.ptr())) + if (nb::isinstance(x) || nb::isinstance(x)) return nb::float_((*obj)(extractdouble(x))); - NumPyArray_DoublePtr xx = extractNumPyDoubleArray(x); - NumPyArray_DoublePtr yy = createNumPyDoubleArrayLike(xx.first); - double* src = xx.second; - double* last = xx.second + PyArray_Size(xx.first.ptr()); - double* dst = yy.second; + NumPyDoubleInput xx = extractNumPyDoubleArray(x); + auto yy = createNumPyDoubleArrayLike(xx); + const double* src = xx.data(); + const double* last = src + xx.size(); + double* dst = yy.array.data(); for (; src != last; ++src, ++dst) *dst = (*obj)(*src); - return yy.first; + return yy.object; } } // namespace nswrap_PDFEnvelope diff --git a/src/extensions/wrap_PairQuantity.cpp b/src/extensions/wrap_PairQuantity.cpp index ea7c2dd..823cad4 100644 --- a/src/extensions/wrap_PairQuantity.cpp +++ b/src/extensions/wrap_PairQuantity.cpp @@ -354,11 +354,9 @@ void set_quantity_slice(QuantityType& values, nb::slice slice, nb::object items) { auto [first, last, step, count] = slice.compute(values.size()); // Finish conversion before editing, including when items aliases values. - // Convert elements as the original vector binding did, so NumPy handles - // byte order and strides instead of using extractQuantityType's raw buffer. QuantityType replacement; - for (nb::handle item : items) - replacement.push_back(nb::cast(item)); + const QuantityType& source = extractQuantityType(items, replacement); + if (&source != &replacement) replacement = source; if (step == 1) { @@ -380,7 +378,7 @@ void set_quantity_slice(QuantityType& values, nb::slice slice, nb::object items) nb::object eval_asarray(PairQuantity& obj, nb::object& a) { - QuantityType value = (a.is_none()) ? obj.eval() : obj.eval(a); + const QuantityType& value = (a.is_none()) ? obj.eval() : obj.eval(a); nb::object rv = convertToNumPyArray(value); return rv; } diff --git a/src/extensions/wrap_ScatteringFactorTable.cpp b/src/extensions/wrap_ScatteringFactorTable.cpp index 91e99d1..2c8f8ac 100644 --- a/src/extensions/wrap_ScatteringFactorTable.cpp +++ b/src/extensions/wrap_ScatteringFactorTable.cpp @@ -26,12 +26,7 @@ #include #include -#include "srreal_numpy_symbol.hpp" -// numpy/arrayobject.h needs to be included after srreal_numpy_symbol.hpp, -// which defines PY_ARRAY_UNIQUE_SYMBOL. NO_IMPORT_ARRAY indicates -// import_array will be called in the extension module initializer. -#define NO_IMPORT_ARRAY -#include + #include "srreal_converters.hpp" @@ -328,18 +323,18 @@ class ScatteringFactorTableWrap : nb::object lookupnparray(const ScatteringFactorTable& sftb, std::string smbl, nb::object& qobj) { - if (PyFloat_Check(qobj.ptr()) || PyLong_Check(qobj.ptr())) + if (nb::isinstance(qobj) || nb::isinstance(qobj)) return nb::float_(sftb.lookup(smbl, extractdouble(qobj))); - NumPyArray_DoublePtr aa = extractNumPyDoubleArray(qobj); - NumPyArray_DoublePtr bb = createNumPyDoubleArrayLike(aa.first); - double* src = aa.second; - double* last = aa.second + PyArray_Size(aa.first.ptr()); - double* dst = bb.second; + NumPyDoubleInput aa = extractNumPyDoubleArray(qobj); + auto bb = createNumPyDoubleArrayLike(aa); + const double* src = aa.data(); + const double* last = src + aa.size(); + double* dst = bb.array.data(); for (; src != last; ++src, ++dst) { *dst = sftb.lookup(smbl, *src); } - return bb.first; + return bb.object; } diff --git a/tests/test_pairquantity.py b/tests/test_pairquantity.py index 14335a9..66758e6 100644 --- a/tests/test_pairquantity.py +++ b/tests/test_pairquantity.py @@ -10,7 +10,7 @@ from diffpy.srreal.pairquantity import PairQuantity from diffpy.srreal.pdfcalculator import PDFCalculator -from diffpy.srreal.srreal_ext import BasePairQuantity +from diffpy.srreal.srreal_ext import BasePairQuantity, StructureDifference # ---------------------------------------------------------------------------- @@ -88,6 +88,70 @@ def test_setPairMask_args(self): self.assertFalse(gpm(2, 7)) return + def test_numpy_index_layouts(self): + """Index conversion follows logical values, not physical storage.""" + values = numpy.arange(6, dtype=numpy.intc) + swapped = values.astype(values.dtype.newbyteorder("S")) + unaligned = numpy.ndarray( + (3,), dtype=numpy.int64, buffer=bytearray(25), offset=1 + ) + unaligned[:] = [0, 2, 4] + for indices in [values[::2], values[::-1], swapped, unaligned]: + difference = StructureDifference() + difference.pop0 = indices + difference.add1 = indices + self.assertEqual(difference.pop0, indices.tolist()) + self.assertEqual(difference.add1, indices.tolist()) + self.pq.setPairMask(indices, 7, True, others=False) + for i in range(6): + self.assertEqual(self.pq.getPairMask(i, 7), i in indices) + + def test_numpy_index_bounds(self): + """Wide integers must not wrap into valid indices.""" + limits = numpy.iinfo(numpy.intc) + difference = StructureDifference() + for dtype in [ + numpy.int8, + numpy.uint8, + numpy.int16, + numpy.uint16, + numpy.int32, + numpy.uint32, + numpy.int64, + numpy.uint64, + ]: + difference.pop0 = numpy.array([0, 1, 3], dtype=dtype) + self.assertEqual(difference.pop0, [0, 1, 3]) + difference.pop0 = numpy.array( + [limits.min, limits.max], dtype=numpy.int64 + ) + self.assertEqual(difference.pop0, [limits.min, limits.max]) + for dtype, values in [ + (numpy.int64, [limits.min - 1, limits.max + 1, 2**32]), + (numpy.uint32, [limits.max + 1, numpy.iinfo(numpy.uint32).max]), + (numpy.uint64, [limits.max + 1, numpy.iinfo(numpy.uint64).max]), + ]: + for value in values: + indices = numpy.array([value], dtype=dtype) + difference.pop0 = [5] + with self.assertRaises(OverflowError): + difference.pop0 = indices + self.assertEqual(difference.pop0, [5]) + with self.assertRaises(OverflowError): + self.pq.setPairMask(indices, 0, True) + + def test_numpy_indices_reject_nonintegers(self): + for indices in [ + numpy.array([1.5]), + numpy.array(["1"]), + numpy.datetime64("1970-01-03"), + numpy.timedelta64(2, "D"), + ]: + with self.assertRaises(TypeError): + StructureDifference().pop0 = indices + with self.assertRaises(TypeError): + self.pq.setPairMask(indices, 0, True) + def test_getStructure(self): """Check PairQuantity.getStructure()""" adpt = self.pq.getStructure() diff --git a/tests/test_pdfcalculator.py b/tests/test_pdfcalculator.py index d6516b5..c8e311e 100644 --- a/tests/test_pdfcalculator.py +++ b/tests/test_pdfcalculator.py @@ -352,6 +352,24 @@ def test_fft_conversions(self): self.assertEqual(dq0, dq2) return + def test_fft_swapped_and_unaligned_buffers(self): + """Normalize FFT input byte order and alignment.""" + values = numpy.arange(4.0) + swapped = values.astype(values.dtype.newbyteorder("S")) + unaligned = numpy.ndarray( + values.shape, + dtype=float, + buffer=bytearray(values.nbytes + 1), + offset=1, + ) + unaligned[:] = values + unaligned.flags.writeable = False + expected, step = fftgtof(values, 0.1) + for source in [swapped, unaligned]: + actual, actual_step = fftgtof(source, 0.1) + numpy.testing.assert_array_equal(actual, expected) + self.assertEqual(actual_step, step) + def test_fft_roundtrip(self): """Check if forward and inverse transformation recover the input.""" diff --git a/tests/test_python_interfaces.py b/tests/test_python_interfaces.py index 8d04450..cf2ca1d 100644 --- a/tests/test_python_interfaces.py +++ b/tests/test_python_interfaces.py @@ -124,3 +124,38 @@ def test_scalar_and_array_return_types(factory, method, prefix): result = function(*prefix, value) assert isinstance(result, np.ndarray) assert result.shape == np.asarray(value).shape + + +def test_elementwise_arrays_preserve_logical_shape_and_values(): + baseline = ext.LinearBaseline() + baseline.slope = 2 + values = np.arange(6.0).reshape(2, 3).T[:, ::-1] + values.flags.writeable = False + result = baseline(values) + np.testing.assert_array_equal(result, values * 2) + assert result.dtype == np.float64 + assert result.flags.c_contiguous and result.flags.writeable + assert result.flags.owndata + assert baseline(np.empty((0, 3))).shape == (0, 3) + + +def test_numpy_casting_distinguishes_scalars_sequences_and_arrays(): + baseline = ext.LinearBaseline() + baseline.slope = 2 + for value in [ + np.longdouble(2), + np.str_("2"), + np.bytes_("2"), + b"2", + np.datetime64("1970-01-03"), + np.timedelta64(2, "D"), + ]: + result = baseline(value) + assert isinstance(result, np.ndarray) and result.shape == () + assert result == 4 + with pytest.warns(np.exceptions.ComplexWarning): + assert baseline(np.complex128(2 + 3j)) == 4 + for dtype in [object, "U2", np.complex128]: + with pytest.raises(TypeError): + baseline(np.array([1, 2], dtype=dtype)) + np.testing.assert_array_equal(baseline(["1", "2"]), [2, 4]) diff --git a/tests/test_quantitytype.py b/tests/test_quantitytype.py index fcaac2e..6c510f9 100644 --- a/tests/test_quantitytype.py +++ b/tests/test_quantitytype.py @@ -16,6 +16,14 @@ def test_quantity_slice_edits_are_atomic_and_support_aliasing(): with pytest.raises(TypeError): values[:] = [1, object()] assert list(values) == [3, 5, 4, 1] + + def failing_values(): + yield 1 + raise RuntimeError("iteration failed") + + with pytest.raises(RuntimeError, match="iteration failed"): + values[:] = failing_values() + assert list(values) == [3, 5, 4, 1] values[:] = [] assert len(values) == 0 @@ -34,3 +42,26 @@ def test_mutable_quantity_uses_value_equality_without_hashing(): assert values != QuantityType([2, 1]) with pytest.raises(TypeError): hash(values) + + +def test_quantity_slices_preserve_swapped_and_reversed_values(): + values = np.arange(4.0).astype(np.dtype(float).newbyteorder("S"))[::-1] + quantity = QuantityType() + quantity[:] = values + assert list(quantity) == values.tolist() + quantity[::-1] = values + assert list(quantity) == values[::-1].tolist() + + +def test_result_arrays_own_independent_writable_storage(): + quantity = PairQuantity() + quantity._value[:] = [1, 2, 3] + first, second = quantity.value, quantity.value + assert first.flags.owndata and first.flags.writeable + assert not np.shares_memory(first, second) + first[:] = -1 + assert list(quantity._value) == [1, 2, 3] + quantity._value[:] = [4, 5] + del quantity + np.testing.assert_array_equal(second, [1, 2, 3]) + first.resize((4,), refcheck=False) diff --git a/tests/test_structure_lifetimes.py b/tests/test_structure_lifetimes.py index 9add0c2..6a7cb32 100644 --- a/tests/test_structure_lifetimes.py +++ b/tests/test_structure_lifetimes.py @@ -2,6 +2,7 @@ import gc import pickle +import weakref import numpy as np import pytest @@ -150,3 +151,24 @@ def clone(self): adapter = Derived() adapter.append(ext.Atom()) assert adapter.clone()[0].xc == 1 + + +@pytest.mark.parametrize("attached", [False, True]) +@pytest.mark.parametrize("field", ["xyz_cartn", "uij_cartn"]) +def test_view_slices_keep_atom_storage_alive_without_python_owners( + attached, field +): + adapter = make_structure() + atom = adapter[0] if attached else ext.Atom() + reference = weakref.ref(atom) + view = getattr(atom, field) + view[...] = 3 + retained = view[..., ::-1] + del view, atom, adapter + gc.collect() + assert reference() is not None + retained[...] = 5 + np.testing.assert_array_equal(retained, 5) + del retained + gc.collect() + assert reference() is None diff --git a/tests/test_structureadapter.py b/tests/test_structureadapter.py index cd22528..6e3cb1e 100644 --- a/tests/test_structureadapter.py +++ b/tests/test_structureadapter.py @@ -627,6 +627,30 @@ def test_uij_cartn(self): self.assertEqual(0.023, a.uc23) return + def test_array_assignment_broadcasts_and_validates_before_writing(self): + values = numpy.arange(3.0)[::-1] + for field, shape in [("xyz_cartn", (3,)), ("uij_cartn", (3, 3))]: + setattr(self.a, field, values) + numpy.testing.assert_array_equal( + getattr(self.a, field), numpy.broadcast_to(values, shape) + ) + setattr(self.a, field, numpy.array(7.0)) + with self.assertRaises(TypeError): + setattr(self.a, field, [1, 2, object()]) + numpy.testing.assert_array_equal(getattr(self.a, field), 7) + matrix = numpy.arange(9.0).reshape(3, 3).T + self.a.uij_cartn = matrix + numpy.testing.assert_array_equal(self.a.uij_cartn, matrix) + + def test_numpy_scalar_casting_preserves_warnings(self): + self.a.xc = numpy.str_("2") + self.assertEqual(self.a.xc, 2) + with self.assertWarns(numpy.exceptions.ComplexWarning): + self.a.xc = numpy.complex128(2 + 3j) + self.assertEqual(self.a.xc, 2) + with self.assertRaises(TypeError): + self.a.xc = "2" + def test_xc_yc_zc(self): "check Atom properties xc, yc, zc." a = self.a From 2355f2184eddd87816241007a82ff21b0f371eb2 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 11 Oct 2026 20:14:29 +0000 Subject: [PATCH 3/3] [pre-commit.ci] auto fixes from pre-commit hooks --- tests/test_pairquantity.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_pairquantity.py b/tests/test_pairquantity.py index 66758e6..e3c121a 100644 --- a/tests/test_pairquantity.py +++ b/tests/test_pairquantity.py @@ -89,7 +89,8 @@ def test_setPairMask_args(self): return def test_numpy_index_layouts(self): - """Index conversion follows logical values, not physical storage.""" + """Index conversion follows logical values, not physical + storage.""" values = numpy.arange(6, dtype=numpy.intc) swapped = values.astype(values.dtype.newbyteorder("S")) unaligned = numpy.ndarray(