improve stubs to limit the number of mypy errors

if .def("str") goes after other methods, it avoids name conflict:
https://mypy.readthedocs.io/en/stable/common_issues.html#dealing-with-conflicting-names
(this documentation describes conflicts of methods with built-in names,
but it doesn't say that it depends on the definition order, perhaps it's
unintended and will be fixed - broken for us - in the future)

use def instead of attr for __matmul__: stubgen seems to handle aliases
of functions and types, but not methods; having def also allows to add
nb::is_operator(), which only affects TypeError message - not sure if
it's better with is_operator()

add explicit signature sig("...") for __eq__ functions,
to avoid mypy [override] error

there are still some mypy override errors left, can be disabled with:
mypy --disable-error-code override ...

and assorted minor edits

.gitignore

@@ -11,6 +11,7 @@ tags
 /.vscode/
 /docs/_build
 /.eggs/
+/wheelhouse/
 .gdb_history
 .*.swp
 
@@ -65,6 +66,7 @@ docs/8xfm.cif
 /wasm/gemmi.js
 /wasm/mtz.js
 /wasm/node_modules
+/wasm/package-lock.json
 
 # files generated by shroud
 fortran/*.json

CMakeLists.txt

@@ -520,15 +520,15 @@ if (USE_PYTHON)
     nanobind_add_stub(
       gemmi_stub
       MODULE gemmi
-      OUTPUT stubs/gemmi/__init__.pyi
+      OUTPUT __init__.pyi
       PYTHON_PATH $<TARGET_FILE_DIR:gemmi_py>
       DEPENDS gemmi_py
-      MARKER_FILE stubs/gemmi/py.typed
+      MARKER_FILE py.typed
     )
     nanobind_add_stub(
       gemmi_cif_stub
       MODULE gemmi.cif
-      OUTPUT stubs/gemmi/cif.pyi
+      OUTPUT cif.pyi
       PYTHON_PATH $<TARGET_FILE_DIR:gemmi_py>
       DEPENDS gemmi_py
     )
@@ -593,7 +593,10 @@ if (USE_PYTHON)
           FILES_MATCHING PATTERN "*.py"
           PATTERN "[._]*" EXCLUDE)
   if (GENERATE_STUBS AND NOT CMAKE_CROSSCOMPILING)
-    install(DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/stubs/gemmi"
-            DESTINATION "${Python_SITEARCH}" COMPONENT py)
+    install(FILES
+            "${CMAKE_CURRENT_BINARY_DIR}/py.typed"
+            "${CMAKE_CURRENT_BINARY_DIR}/__init__.pyi"
+            "${CMAKE_CURRENT_BINARY_DIR}/cif.pyi"
+            DESTINATION "${Python_SITEARCH}/gemmi" COMPONENT py)
   endif()
 endif()

README.md

@@ -28,4 +28,4 @@ two major providers of software for macromolecular crystallography.
 Citing: [JOSS paper](https://doi.org/10.21105/joss.04200).
 
 License: MPLv2, or (at your option) LGPLv3.
-© 2017-2023 Global Phasing Ltd.
+© 2017-2024 Global Phasing Ltd.

python/cif.cpp

@@ -382,7 +382,6 @@ void add_cif(nb::module_& cif) {
     .def("__setitem__", [](Column &self, int idx, std::string value) {
         self.at(idx) = value;
     })
-    .def("str", &Column::str, nb::arg("index"))
     .def("erase", &Column::erase)
     .def("__repr__", [](const Column &self) {
         std::string s = "<gemmi.cif.Column ";
@@ -391,7 +390,8 @@ void add_cif(nb::module_& cif) {
         else
           s += "nil>";
         return s;
-    });
+    })
+    .def("str", &Column::str, nb::arg("index"));
 
   cif_table
     .def("width", &Table::width)
@@ -431,7 +431,6 @@ void add_cif(nb::module_& cif) {
 
   cif_table_row
     .def_ro("row_index", &Table::Row::row_index)
-    .def("str", &Table::Row::str)
     .def("__len__", &Table::Row::size)
     .def("__getitem__", (std::string& (Table::Row::*)(int)) &Table::Row::at)
     .def("__getitem__", [](Table::Row &self, const std::string& tag) {
@@ -455,7 +454,8 @@ void add_cif(nb::module_& cif) {
         for (int i = 0; (size_t)i != self.size(); ++i)
           items += " " + (self.has(i) ? self[i] : "None");
         return "<gemmi.cif.Table.Row:" + items + ">";
-    });
+    })
+    .def("str", &Table::Row::str);
 
   cif.def("quote", &quote, nb::arg("string"));
   cif.def("quote_list", &quote_list);

python/elem.cpp

@@ -76,7 +76,7 @@ void add_elem(nb::module_& m) {
   nb::class_<Element>(m, "Element")
     .def(nb::init<const std::string &>())
     .def(nb::init<int>())
-    .def(nb::self == nb::self)
+    .def(nb::self == nb::self, nb::sig("def __eq__(self, arg: object, /) -> bool"))
     .def_prop_ro("name", &Element::name)
     .def_prop_ro("weight", &Element::weight)
     .def_prop_ro("covalent_r", &Element::covalent_r)

python/meta.cpp

@@ -40,7 +40,7 @@ void add_meta(nb::module_& m) {
     .def("__repr__", [](const SeqId& self) {
         return "<gemmi.SeqId " + self.str() + ">";
     })
-    .def(nb::self == nb::self)
+    .def(nb::self == nb::self, nb::sig("def __eq__(self, arg: object, /) -> bool"))
     .def(nb::self < nb::self)
     .def("__hash__", [](const SeqId& self) {
         // TODO: implement in hash<gemmi::SeqId> in seqid.hpp?
@@ -57,7 +57,7 @@ void add_meta(nb::module_& m) {
     .def("__repr__", [](const ResidueId& self) {
         return "<gemmi.ResidueId " + self.str() + ">";
     })
-    .def(nb::self == nb::self)
+    .def(nb::self == nb::self, nb::sig("def __eq__(self, arg: object, /) -> bool"))
     ;
 
   nb::class_<AtomAddress>(m, "AtomAddress")
@@ -74,7 +74,7 @@ void add_meta(nb::module_& m) {
     .def("__repr__", [](const AtomAddress& self) {
         return cat("<gemmi.AtomAddress ", self.str(), '>');
     })
-    .def(nb::self == nb::self)
+    .def(nb::self == nb::self, nb::sig("def __eq__(self, arg: object, /) -> bool"))
     ;
 
   // metadata.hpp

python/mol.cpp

@@ -585,7 +585,6 @@ void add_mol(nb::module_& m) {
          nb::keep_alive<1, 2>())
     .def("first", &Selection::first, nb::rv_policy::reference,
          nb::keep_alive<1, 2>())
-    .def("str", &Selection::str)
     .def("set_residue_flags", &Selection::set_residue_flags)
     .def("set_atom_flags", &Selection::set_atom_flags)
     .def("copy_model_selection", &Selection::copy_selection<Model>)
@@ -596,7 +595,8 @@ void add_mol(nb::module_& m) {
     .def("remove_not_selected", &Selection::remove_not_selected<Model>)
     .def("__repr__", [](const Selection& self) {
         return "<gemmi.Selection CID: " + self.str() + ">";
-    });
+    })
+    .def("str", &Selection::str);
 
   pySelectionModelsProxy
     .def("__iter__", [](FilterProxy<Selection, Model>& self) {

python/sym.cpp

@@ -74,7 +74,7 @@ void add_symmetry(nb::module_& m) {
          nb::is_operator())
     .def(nb::self == nb::self)
     .def("__eq__", [](const Op& a, const std::string& b) { return a == parse_triplet(b); },
-         nb::is_operator())
+         nb::is_operator(), nb::sig("def __eq__(self, arg: object, /) -> bool"))
     .def("__copy__", [](const Op& self) { return Op(self); })
     .def("__deepcopy__", [](const Op& self, nb::dict) { return Op(self); }, nb::arg("memo"))
     .def("__hash__", [](const Op& self) { return std::hash<Op>()(self); })
@@ -98,7 +98,7 @@ void add_symmetry(nb::module_& m) {
         return nb::make_iterator(nb::type<GroupOps>(), "iterator", self);
     }, nb::keep_alive<0, 1>())
     .def("__eq__", [](const GroupOps &a, const GroupOps &b) { return a.is_same_as(b); },
-         nb::is_operator())
+         nb::is_operator(), nb::sig("def __eq__(self, arg: object, /) -> bool"))
     .def("__len__", [](const GroupOps& g) { return g.order(); })
     .def("__deepcopy__", [](const GroupOps& g, nb::dict) { return GroupOps(g); },
          nb::arg("memo"))
@@ -202,7 +202,7 @@ void add_symmetry(nb::module_& m) {
     // (see space group 65 in the ITB list) are considered equal.
     .def("__eq__", [](const SpaceGroup& a, const SpaceGroup& b) {
         return strcmp(a.hall, b.hall) == 0;
-    }, nb::is_operator())
+    }, nb::is_operator(), nb::sig("def __eq__(self, arg: object, /) -> bool"))
     .def("__reduce__", [](const SpaceGroup& self) {
         // faster than just serializing self.xhm(), but also more tricky
         std::ptrdiff_t pos = &self - spacegroup_tables::main;

python/topo.cpp

@@ -6,6 +6,7 @@
 
 #include "common.h"
 #include <nanobind/stl/bind_vector.h>
+#include <nanobind/stl/array.h>   // for Topo::Bond::atoms
 #include <nanobind/stl/string.h>
 #include <nanobind/stl/vector.h>  // for find_missing_atoms
 #include <nanobind/stl/unique_ptr.h>  // for prepare_topology

python/unitcell.cpp

@@ -144,6 +144,8 @@ void add_unitcell(nb::module_& m) {
     .def(nb::self - nb::self)
     .def("multiply", (Mat33 (Mat33::*)(const Mat33&) const) &Mat33::multiply)
     .def("multiply", (Vec3 (Mat33::*)(const Vec3&) const) &Mat33::multiply)
+    .def("__matmul__", (Mat33 (Mat33::*)(const Mat33&) const) &Mat33::multiply, nb::is_operator())
+    .def("__matmul__", (Vec3 (Mat33::*)(const Vec3&) const) &Mat33::multiply, nb::is_operator())
     .def("left_multiply", &Mat33::left_multiply)
     .def("multiply_by_diagonal", &Mat33::multiply_by_diagonal)
     .def("transpose", &Mat33::transpose)
@@ -164,7 +166,6 @@ void add_unitcell(nb::module_& m) {
                "             [" + triple(a[1][0], a[1][1], a[1][2]) + "]\n"
                "             [" + triple(a[2][0], a[2][1], a[2][2]) + "]>";
     });
-  mat33.attr("__matmul__") = mat33.attr("multiply");
 
   add_smat33<double>(m, "SMat33d");
   add_smat33<float>(m, "SMat33f");
@@ -182,9 +183,9 @@ void add_unitcell(nb::module_& m) {
     .def("inverse", &Transform::inverse)
     .def("apply", &Transform::apply)
     .def("combine", &Transform::combine)
+    .def("__matmul__", &Transform::combine, nb::is_operator())
     .def("is_identity", &Transform::is_identity)
     .def("approx", &Transform::approx, nb::arg("other"), nb::arg("epsilon"));
-  transform.attr("__matmul__") = transform.attr("combine");
 
   nb::class_<Position, Vec3>(m, "Position")
     .def(nb::init<double,double,double>())
@@ -319,7 +320,7 @@ void add_unitcell(nb::module_& m) {
     .def("reciprocal", &UnitCell::reciprocal)
     .def("get_hkl_limits", &UnitCell::get_hkl_limits, nb::arg("dmin"))
     .def("primitive_orth_matrix", &UnitCell::primitive_orth_matrix, nb::arg("centring_type"))
-    .def(nb::self == nb::self)
+    .def(nb::self == nb::self, nb::sig("def __eq__(self, arg: object, /) -> bool"))
     .def("__getstate__", &getstate<UnitCell>)
     .def("__setstate__", &setstate<UnitCell>)
     .def("__repr__", [](const UnitCell& self) {

run-tests.sh

@@ -45,6 +45,7 @@ if [ $# = 0 ] || [ $1 != i ]; then
     export PATH="$BUILD_DIR:$PATH"
 fi
 $PYTHON -m unittest discover -s tests
+grep :: $BUILD_DIR/*.pyi ||:
 
 if [ -z "${NO_DOCTEST-}" ]; then
     # 'make doctest' works only if sphinx-build was installed for python3.