fix: return numpy arrays from CrystVector/CrystMatrix bindings

news/128.rst

@@ -0,0 +1,28 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* <news item>
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* Return numpy arrays from ``UnitCell.GetLatticePar()``,
+  ``UnitCell.GetOrthonormalCoords()``, ``Crystal.GetMinDistanceTable()``
+  and the ``ScatteringPower`` methods ``GetScatteringFactor()``,
+  ``GetTemperatureFactor()``, ``GetResonantScattFactReal()`` and
+  ``GetResonantScattFactImag()``, which raised a ``TypeError`` in the
+  nanobind bindings.
+
+**Security:**
+
+* <news item>

src/extensions/helpers_nb.hpp

@@ -477,18 +477,28 @@ void bind_refinableobj_forwarding(nb::class_<T, Extra...>& cls)
         ;
 }
 
+namespace ObjCryst { class ScatteringData; }
+
 template <typename T, typename... Extra>
 void bind_scatteringpower_forwarding(nb::class_<T, Extra...>& cls)
 {
     cls
-        .def("GetScatteringFactor",  &T::GetScatteringFactor,
+        .def("GetScatteringFactor",
+             [](T& sp, const ObjCryst::ScatteringData& data, const int idx){
+                 return crystvec_to_array(sp.GetScatteringFactor(data, idx)); },
              nb::arg("data"), nb::arg("spgSymPosIndex") = -1)
         .def("GetForwardScatteringFactor", &T::GetForwardScatteringFactor)
-        .def("GetTemperatureFactor", &T::GetTemperatureFactor,
+        .def("GetTemperatureFactor",
+             [](T& sp, const ObjCryst::ScatteringData& data, const int idx){
+                 return crystvec_to_array(sp.GetTemperatureFactor(data, idx)); },
              nb::arg("data"), nb::arg("spgSymPosIndex") = -1)
-        .def("GetResonantScattFactReal", &T::GetResonantScattFactReal,
+        .def("GetResonantScattFactReal",
+             [](T& sp, const ObjCryst::ScatteringData& data, const int idx){
+                 return crystmat_to_array(sp.GetResonantScattFactReal(data, idx)); },
              nb::arg("data"), nb::arg("spgSymPosIndex") = -1)
-        .def("GetResonantScattFactImag", &T::GetResonantScattFactImag,
+        .def("GetResonantScattFactImag",
+             [](T& sp, const ObjCryst::ScatteringData& data, const int idx){
+                 return crystmat_to_array(sp.GetResonantScattFactImag(data, idx)); },
              nb::arg("data"), nb::arg("spgSymPosIndex") = -1)
         .def("IsScatteringFactorAnisotropic",  &T::IsScatteringFactorAnisotropic)
         .def("IsTemperatureFactorAnisotropic", &T::IsTemperatureFactorAnisotropic)

src/extensions/nb_crystal.cpp

@@ -284,7 +284,8 @@ void wrap_crystal(nb::module_& m)
         .def("GetScatteringComponentList", &_GetScatteringComponentList)
         .def("GetClockScattCompList", &Crystal::GetClockScattCompList,
              nb::rv_policy::reference_internal)
-        .def("GetMinDistanceTable", &Crystal::GetMinDistanceTable,
+        .def("GetMinDistanceTable",
+             [](const Crystal& c, double d){ return crystmat_to_array(c.GetMinDistanceTable(d)); },
              nb::arg("minDistance") = 1.0)
         .def("PrintMinDistanceTable",
              [](const Crystal& c, double d){ c.PrintMinDistanceTable(d); },

src/extensions/nb_scatteringpower.cpp

@@ -76,14 +76,22 @@ void wrap_scatteringpower(nb::module_& m)
 
     nb::class_<ScatteringPower, PyScatteringPower> cls(m, "ScatteringPower");
     cls
-        .def("GetScatteringFactor",  &ScatteringPower::GetScatteringFactor,
+        .def("GetScatteringFactor",
+             [](ScatteringPower& sp, const ScatteringData& data, const int idx){
+                 return crystvec_to_array(sp.GetScatteringFactor(data, idx)); },
              nb::arg("data"), nb::arg("spgSymPosIndex") = -1)
         .def("GetForwardScatteringFactor", &ScatteringPower::GetForwardScatteringFactor)
-        .def("GetTemperatureFactor", &ScatteringPower::GetTemperatureFactor,
+        .def("GetTemperatureFactor",
+             [](ScatteringPower& sp, const ScatteringData& data, const int idx){
+                 return crystvec_to_array(sp.GetTemperatureFactor(data, idx)); },
              nb::arg("data"), nb::arg("spgSymPosIndex") = -1)
-        .def("GetResonantScattFactReal", &ScatteringPower::GetResonantScattFactReal,
+        .def("GetResonantScattFactReal",
+             [](ScatteringPower& sp, const ScatteringData& data, const int idx){
+                 return crystmat_to_array(sp.GetResonantScattFactReal(data, idx)); },
              nb::arg("data"), nb::arg("spgSymPosIndex") = -1)
-        .def("GetResonantScattFactImag", &ScatteringPower::GetResonantScattFactImag,
+        .def("GetResonantScattFactImag",
+             [](ScatteringPower& sp, const ScatteringData& data, const int idx){
+                 return crystmat_to_array(sp.GetResonantScattFactImag(data, idx)); },
              nb::arg("data"), nb::arg("spgSymPosIndex") = -1)
         .def("IsScatteringFactorAnisotropic",  &ScatteringPower::IsScatteringFactorAnisotropic)
         .def("IsTemperatureFactorAnisotropic", &ScatteringPower::IsTemperatureFactorAnisotropic)

src/extensions/nb_unitcell.cpp

@@ -93,7 +93,7 @@ void wrap_unitcell(nb::module_& m)
                       const std::string&>())
         .def(nb::init<const UnitCell&>())
         .def("GetLatticePar",
-             nb::overload_cast<>(&UnitCell::GetLatticePar, nb::const_))
+             [](const UnitCell& u){ return crystvec_to_array(u.GetLatticePar()); })
         .def("GetLatticePar",
              nb::overload_cast<const int>(&UnitCell::GetLatticePar, nb::const_))
         .def("GetClockLatticePar",   &UnitCell::GetClockLatticePar,
@@ -102,7 +102,9 @@ void wrap_unitcell(nb::module_& m)
         .def("GetOrthMatrix",  [](UnitCell& u){ return crystmat_to_array(u.GetOrthMatrix()); })
         .def("GetClockMetricMatrix", &UnitCell::GetClockMetricMatrix,
              nb::rv_policy::reference_internal)
-        .def("GetOrthonormalCoords", &UnitCell::GetOrthonormalCoords)
+        .def("GetOrthonormalCoords",
+             [](const UnitCell& u, double x, double y, double z){
+                 return crystvec_to_array(u.GetOrthonormalCoords(x, y, z)); })
         .def("FractionalToOrthonormalCoords", &FractionalToOrthonormal)
         .def("OrthonormalToFractionalCoords", &OrthonormalToFractional)
         .def("MillerToOrthonormalCoords",     &MillerToOrthonormal)

tests/test_array_returns.py

@@ -0,0 +1,117 @@
+#!/usr/bin/env python
+"""Tests for bindings that return ObjCryst's CrystVector / CrystMatrix.
+
+nanobind has no global converter for these types: a binding that returns
+them without ``crystvec_to_array`` / ``crystmat_to_array`` fails only
+when called, with "Unable to convert function return value to a Python
+type".
+"""
+
+import unittest
+
+import numpy as np
+
+import pyobjcryst._pyobjcryst as _pyobjcryst
+from pyobjcryst.atom import Atom
+from pyobjcryst.crystal import Crystal
+from pyobjcryst.powderpattern import PowderPattern
+from pyobjcryst.scatteringpower import ScatteringPowerAtom
+from pyobjcryst.scatteringpowersphere import ScatteringPowerSphere
+
+
+def _corundum():
+    crystal = Crystal(4.76, 4.76, 12.99, "R-3c")
+    al = ScatteringPowerAtom("Al", "Al")
+    o = ScatteringPowerAtom("O", "O")
+    crystal.AddScatteringPower(al)
+    crystal.AddScatteringPower(o)
+    crystal.AddScatterer(Atom(0.0, 0.0, 0.35216, "Al1", al))
+    crystal.AddScatterer(Atom(0.3064, 0.0, 0.25, "O1", o))
+    return crystal, al
+
+
+class TestNoRawCrystVectorReturns(unittest.TestCase):
+
+    def test_signatures(self):
+        """No binding's signature returns an unconverted C++ type."""
+        raw = []
+        for class_name, cls in vars(_pyobjcryst).items():
+            if not isinstance(cls, type):
+                continue
+            for name, member in vars(cls).items():
+                doc = getattr(member, "__doc__", None) or ""
+                for line in doc.splitlines():
+                    if "->" not in line:
+                        continue
+                    return_type = line.rsplit("->", 1)[-1]
+                    if "CrystVector" in return_type or (
+                        "CrystMatrix" in return_type
+                    ):
+                        raw.append(f"{class_name}.{name}: {line.strip()}")
+        self.assertEqual(raw, [])
+
+
+class TestArrayReturns(unittest.TestCase):
+
+    def setUp(self):
+        self.crystal, self.al = _corundum()
+
+    def test_GetLatticePar(self):
+        # b and gamma are derived from the R-3c constraints
+        self.crystal.a = 5.0
+        lattice = self.crystal.GetLatticePar()
+        self.assertIsInstance(lattice, np.ndarray)
+        expected = [self.crystal.GetLatticePar(i) for i in range(6)]
+        np.testing.assert_allclose(lattice, expected)
+        np.testing.assert_allclose(
+            lattice, [5.0, 5.0, 12.99, np.pi / 2, np.pi / 2, 2 * np.pi / 3]
+        )
+
+    def test_GetOrthonormalCoords(self):
+        xyz = self.crystal.GetOrthonormalCoords(0.1, 0.2, 0.3)
+        self.assertIsInstance(xyz, np.ndarray)
+        np.testing.assert_allclose(
+            xyz, self.crystal.FractionalToOrthonormalCoords(0.1, 0.2, 0.3)
+        )
+
+    def test_GetMinDistanceTable(self):
+        table = self.crystal.GetMinDistanceTable(1.0)
+        self.assertIsInstance(table, np.ndarray)
+        self.assertEqual(table.shape, (2, 2))
+        np.testing.assert_allclose(table, table.T)
+        # shortest Al-O distance in corundum
+        self.assertAlmostEqual(table[0, 1], 1.85, places=2)
+
+    def test_scattering_power_factors(self):
+        pp = PowderPattern()
+        pp.SetWavelength(1.0)
+        pp.SetPowderPatternX(np.radians(np.linspace(10.0, 60.0, 200)))
+        pp.AddPowderPatternDiffraction(self.crystal)
+        data = pp.get_crystalline_components()[0]
+        pp.GetPowderPatternCalc()
+        n_refl = data.GetNbRefl()
+        self.assertGreater(n_refl, 0)
+
+        # ScatteringPowerAtom uses the forwarding bindings,
+        # ScatteringPowerSphere the ScatteringPower base bindings
+        sphere = ScatteringPowerSphere("sphere", 2.0, 1.0)
+        for sp in (self.al, sphere):
+            with self.subTest(type(sp).__name__):
+                for name in ("GetScatteringFactor", "GetTemperatureFactor"):
+                    values = getattr(sp, name)(data)
+                    self.assertIsInstance(values, np.ndarray)
+                    self.assertEqual(values.shape, (n_refl,))
+                    self.assertTrue(np.all(np.isfinite(values)))
+                for name in (
+                    "GetResonantScattFactReal",
+                    "GetResonantScattFactImag",
+                ):
+                    values = getattr(sp, name)(data)
+                    self.assertIsInstance(values, np.ndarray)
+                    self.assertEqual(values.ndim, 2)
+
+        self.assertGreater(self.al.GetScatteringFactor(data)[0], 0.0)
+
+
+if __name__ == "__main__":
+    unittest.main()