From 47d532d28690c82bf4385a58f76f25780e3cf1ee Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Mon, 4 Mar 2024 19:36:47 +0100 Subject: [PATCH 1/2] allow arbitrary Python objects to function as handlers --- lib/osmium.cc | 40 ++++++++++-------- lib/python_handler.h | 97 ++++++++++++++++++++++++++++++++++++++++++++ test/test_osmium.py | 28 ++++++++++++- 3 files changed, 145 insertions(+), 20 deletions(-) create mode 100644 lib/python_handler.h diff --git a/lib/osmium.cc b/lib/osmium.cc index 30e70cd4..2fc43a83 100644 --- a/lib/osmium.cc +++ b/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 &&handlers) - : m_handlers(handlers) - {} + HandlerChain(py::args args) + { + m_python_handlers.reserve(args.size()); + for (auto &arg: args) { + if (py::isinstance(arg)) { + // Already a handler object, push back directly. + m_handlers.push_back(arg.cast()); + } 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 m_handlers; + std::vector m_python_handlers; }; -static HandlerChain make_handler_chain(py::args args) -{ - std::vector handlers; - for (auto const &arg: args) { - if (py::isinstance(arg)) { - handlers.push_back(arg.cast()); - } else { - throw py::type_error{"Argument must be a handler-like object."}; - } - } - - return HandlerChain(std::move(handlers)); -} - PYBIND11_MODULE(_osmium, m) { py::register_exception(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); diff --git a/lib/python_handler.h b/lib/python_handler.h new file mode 100644 index 00000000..b546fc1c --- /dev/null +++ b/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 and others. + * For a full list of authors see the git log. + */ +#ifndef PYOSMIUM_PYTHON_HANDLER_HPP +#define PYOSMIUM_PYTHON_HANDLER_HPP + +#include + +#include "base_handler.h" + +namespace pyosmium { + +template +class ObjectGuard { + using WardPtr = T*; + + public: + ObjectGuard(pybind11::object ward) : m_ward(ward) {} + + ~ObjectGuard() { + m_ward.attr("_pyosmium_data").template cast()->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 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 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 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 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 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 + diff --git a/test/test_osmium.py b/test/test_osmium.py index c9c0c333..be38575b 100644 --- a/test/test_osmium.py +++ b/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'] From 52fbab7b83fb3b32ce18cab684fb7943bd880ad5 Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Mon, 4 Mar 2024 19:49:00 +0100 Subject: [PATCH 2/2] actions: update more action scripts --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b40bed37..1d533a24 100644 --- a/.github/workflows/ci.yml +++ b/.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