Merge pull request #242 from lonvia/handlers-from-any-python-class

Allow any Python object to be used as a handler object

.github/workflows/ci.yml

@@ -90,7 +90,7 @@ jobs:
               shell: bash
 
             - name: Upload Artifact
-              uses: actions/upload-artifact@v3
+              uses: actions/upload-artifact@v4
               with:
                   name: pyosmium-linux-x64-dist
                   path: dist
@@ -112,7 +112,7 @@ jobs:
               with:
                   python-version: ${{ matrix.python-version }}
 
-            - uses: actions/download-artifact@v3
+            - uses: actions/download-artifact@v4
               with:
                   name: pyosmium-linux-x64-dist
 
@@ -307,7 +307,7 @@ jobs:
                   CMAKE_TOOLCHAIN_FILE: C:/vcpkg/scripts/buildsystems/vcpkg.cmake
 
             - name: 'Upload Artifact'
-              uses: actions/upload-artifact@v3
+              uses: actions/upload-artifact@v4
               with:
                   name: pyosmium-win64-dist
                   path: dist
@@ -332,7 +332,7 @@ jobs:
               with:
                   python-version: ${{ matrix.python-version }}
 
-            - uses: actions/download-artifact@v3
+            - uses: actions/download-artifact@v4
               with:
                   name: pyosmium-win64-dist
 

lib/osmium.cc

@@ -14,15 +14,32 @@
 
 #include "simple_handler.h"
 #include "osmium_module.h"
+#include "python_handler.h"
 
 namespace py = pybind11;
 
 class HandlerChain : public osmium::handler::Handler
 {
 public:
-    HandlerChain(std::vector<BaseHandler *> &&handlers)
-    : m_handlers(handlers)
-    {}
+    HandlerChain(py::args args)
+    {
+        m_python_handlers.reserve(args.size());
+        for (auto &arg: args) {
+            if (py::isinstance<BaseHandler>(arg)) {
+                // Already a handler object, push back directly.
+                m_handlers.push_back(arg.cast<BaseHandler *>());
+            } else if (py::hasattr(arg, "node") || py::hasattr(arg, "way")
+                       || py::hasattr(arg, "relation")
+                       || py::hasattr(arg, "changeset") || py::hasattr(arg, "area")) {
+                // Python object that looks like a handler.
+                // Wrap into a osmium handler object.
+                m_python_handlers.emplace_back(arg);
+                m_handlers.push_back(&m_python_handlers.back());
+            } else {
+                throw py::type_error{"Argument must be a handler-like object."};
+            }
+        }
+    }
 
     void node(osmium::Node const &o) {
         for (auto const &handler : m_handlers) {
@@ -56,23 +73,10 @@ class HandlerChain : public osmium::handler::Handler
 
 private:
     std::vector<BaseHandler *> m_handlers;
+    std::vector<pyosmium::PythonHandler> m_python_handlers;
 };
 
 
-static HandlerChain make_handler_chain(py::args args)
-{
-    std::vector<BaseHandler *> handlers;
-    for (auto const &arg: args) {
-        if (py::isinstance<BaseHandler>(arg)) {
-            handlers.push_back(arg.cast<BaseHandler *>());
-        } else {
-            throw py::type_error{"Argument must be a handler-like object."};
-        }
-    }
-
-    return HandlerChain(std::move(handlers));
-}
-
 PYBIND11_MODULE(_osmium, m) {
     py::register_exception<osmium::invalid_location>(m, "InvalidLocationError");
     py::register_exception_translator([](std::exception_ptr p) {
@@ -89,7 +93,7 @@ PYBIND11_MODULE(_osmium, m) {
           "Apply a single handler.");
     m.def("apply", [](osmium::io::Reader &rd, py::args args)
                      {
-                         auto handler = make_handler_chain(args);
+                         HandlerChain handler{args};
                          {
                              py::gil_scoped_release release;
                              osmium::apply(rd, handler);

lib/python_handler.h

@@ -0,0 +1,97 @@
+/* SPDX-License-Identifier: BSD-2-Clause
+ *
+ * This file is part of pyosmium. (https://osmcode.org/pyosmium/)
+ *
+ * Copyright (C) 2024 Sarah Hoffmann <[email protected]> and others.
+ * For a full list of authors see the git log.
+ */
+#ifndef PYOSMIUM_PYTHON_HANDLER_HPP
+#define PYOSMIUM_PYTHON_HANDLER_HPP
+
+#include <pybind11/pybind11.h>
+
+#include "base_handler.h"
+
+namespace pyosmium {
+
+template <typename T>
+class ObjectGuard {
+    using WardPtr = T*;
+
+    public:
+        ObjectGuard(pybind11::object ward) : m_ward(ward) {}
+
+        ~ObjectGuard() {
+            m_ward.attr("_pyosmium_data").template cast<WardPtr>()->invalidate();
+        }
+
+    private:
+        pybind11::object m_ward;
+};
+
+
+class PythonHandler : public BaseHandler
+{
+public:
+    PythonHandler(pybind11::handle handler)
+    : m_handler(std::move(handler))
+    {}
+
+    void node(osmium::Node const *n) override
+    {
+        pybind11::gil_scoped_acquire acquire;
+        if (pybind11::hasattr(m_handler, "node")) {
+            auto obj = m_type_module.attr("Node")(COSMNode{n});
+            ObjectGuard<COSMNode> guard(obj);
+            m_handler.attr("node")(obj);
+        }
+    }
+
+    void way(osmium::Way *w) override
+    {
+        pybind11::gil_scoped_acquire acquire;
+        if (pybind11::hasattr(m_handler, "way")) {
+            auto obj = m_type_module.attr("Way")(COSMWay{w});
+            ObjectGuard<COSMWay> guard(obj);
+            m_handler.attr("way")(obj);
+        }
+    }
+
+    void relation(osmium::Relation const *r) override
+    {
+        pybind11::gil_scoped_acquire acquire;
+        if (pybind11::hasattr(m_handler, "relation")) {
+            auto obj = m_type_module.attr("Relation")(COSMRelation{r});
+            ObjectGuard<COSMRelation> guard(obj);
+            m_handler.attr("relation")(obj);
+        }
+    }
+
+    void changeset(osmium::Changeset const *c) override
+    {
+        pybind11::gil_scoped_acquire acquire;
+        if (pybind11::hasattr(m_handler, "changeset")) {
+            auto obj = m_type_module.attr("Changeset")(COSMChangeset{c});
+            ObjectGuard<COSMChangeset> guard(obj);
+            m_handler.attr("changeset")(obj);
+        }
+    }
+
+    void area(osmium::Area const *a) override
+    {
+        pybind11::gil_scoped_acquire acquire;
+        if (pybind11::hasattr(m_handler, "area")) {
+            auto obj = m_type_module.attr("Area")(COSMArea{a});
+            ObjectGuard<COSMArea> guard(obj);
+            m_handler.attr("area")(obj);
+        }
+    }
+private:
+    pybind11::object m_type_module = pybind11::module_::import("osmium.osm.types");
+    pybind11::handle m_handler;
+};
+
+} // namespace
+
+#endif //PYOSMIUM_PYTHON_HANDLER_HPP
+

test/test_osmium.py

@@ -34,9 +34,8 @@ def test_apply_node_location_handler(opl_reader, ignore_error):
     if ignore_error:
         hdlr.ignore_errors()
 
-    class WayNodeHandler(o.SimpleHandler):
+    class WayNodeHandler:
         def __init__(self):
-            super().__init__()
             self.collect = []
             self.with_error = []
 
@@ -67,3 +66,28 @@ def way(self, w):
     else:
         with pytest.raises(osmium.InvalidLocationError):
             o.apply(opl.reader(data), hdlr, tester)
+
+
+def test_apply_invalid_handler_object(opl_reader):
+    class DummyHandler:
+        def some_func():
+            print('A')
+
+    with pytest.raises(TypeError):
+        o.apply(opl_reader("n1 x2 z4"), DummyHandler())
+
+
+def test_mixed_handlers(opl_reader):
+    logged = []
+
+    class OldStyle(o.SimpleHandler):
+        def node(self, n):
+            logged.append('old')
+
+    class NewStyle:
+        def node(self, n):
+            logged.append('new')
+
+    o.apply(opl_reader("n1 x0 y0"), NewStyle(), OldStyle(), NewStyle(), OldStyle())
+
+    assert logged == ['new', 'old', 'new', 'old']