From 65d56e056dfc1c1cc8821c7c4d8fa9f594d93677 Mon Sep 17 00:00:00 2001 From: Richard Frank Date: Tue, 19 May 2020 16:54:53 -0400 Subject: [PATCH 1/2] BUG: Update from_object dispatch for table_view to use borrowed_ref, which is the new expected interface --- include/libpy/table.h | 34 +++++++++++++++++----------------- tests/test_from_object.cc | 28 ++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/include/libpy/table.h b/include/libpy/table.h index 43a276f1..7a6ac140 100644 --- a/include/libpy/table.h +++ b/include/libpy/table.h @@ -1140,18 +1140,18 @@ struct from_object> { using type = py::table_view; template - static auto pop_column(PyObject* t) { + static auto pop_column(py::borrowed_ref<> t) { auto text = py::cs::to_array(typename Column::key{}); auto column_name = py::to_object( *reinterpret_cast*>(text.data())); if (!column_name) { throw py::exception(); } - PyObject* column_ob = PyDict_GetItem(t, column_name.get()); + PyObject* column_ob = PyDict_GetItem(t.get(), column_name.get()); if (!column_ob) { throw py::exception(PyExc_ValueError, "missing column: ", column_name); } - if (PyDict_DelItem(t, column_name.get())) { + if (PyDict_DelItem(t.get(), column_name.get())) { // pop the item to track which columns we used throw py::exception(); } @@ -1159,20 +1159,20 @@ struct from_object> { } public: - static type f(PyObject* t) { - if (!PyDict_Check(t)) { + static type f(py::borrowed_ref<> t) { + if (!PyDict_Check(t.get())) { throw py::exception( PyExc_TypeError, "from_object> input must be a Python dictionary, got: ", - Py_TYPE(t)->tp_name); + Py_TYPE(t.get())->tp_name); } - py::owned_ref copy(PyDict_Copy(t)); + py::owned_ref copy(PyDict_Copy(t.get())); if (!copy) { throw py::exception(); } - type out(pop_column>(copy.get())...); + type out(pop_column>(copy)...); if (PyDict_Size(copy.get())) { py::owned_ref keys(PyDict_Keys(copy.get())); if (!keys) { @@ -1190,18 +1190,18 @@ struct from_object> { using type = py::row; template - static auto pop_column(PyObject* t) { + static auto pop_column(py::borrowed_ref<> t) { auto text = py::cs::to_array(typename Column::key{}); auto column_name = py::to_object( *reinterpret_cast*>(text.data())); if (!column_name) { throw py::exception(); } - PyObject* column_ob = PyDict_GetItem(t, column_name.get()); + PyObject* column_ob = PyDict_GetItem(t.get(), column_name.get()); if (!column_ob) { throw py::exception(PyExc_ValueError, "missing column: ", column_name); } - if (PyDict_DelItem(t, column_name.get())) { + if (PyDict_DelItem(t.get(), column_name.get())) { // pop the item to track which columns we used throw py::exception(); } @@ -1209,20 +1209,20 @@ struct from_object> { } public: - static type f(PyObject* t) { - if (!PyDict_Check(t)) { + static type f(py::borrowed_ref<> t) { + if (!PyDict_Check(t.get())) { throw py::exception( PyExc_TypeError, - "from_object> input must be a Python dictionary, got: ", - Py_TYPE(t)->tp_name); + "from_object> input must be a Python dictionary, got: ", + Py_TYPE(t.get())->tp_name); } - py::owned_ref copy(PyDict_Copy(t)); + py::owned_ref copy(PyDict_Copy(t.get())); if (!copy) { throw py::exception(); } - type out(pop_column>(copy.get())...); + type out(pop_column>(copy)...); if (PyDict_Size(copy.get())) { py::owned_ref keys(PyDict_Keys(copy.get())); if (!keys) { diff --git a/tests/test_from_object.cc b/tests/test_from_object.cc index f9109db9..712a707a 100644 --- a/tests/test_from_object.cc +++ b/tests/test_from_object.cc @@ -9,11 +9,14 @@ #include "libpy/numpy_utils.h" #include "libpy/object_map_key.h" #include "libpy/owned_ref.h" +#include "libpy/table.h" #include "libpy/util.h" #include "test_utils.h" namespace test_from_object { +using namespace py::cs::literals; + class from_object : public with_python_interpreter {}; template @@ -243,6 +246,31 @@ TEST_F(from_object, ndarray_view_any_ref) { } } +TEST_F(from_object, table_view) { + py::owned_ref ns = RUN_PYTHON(R"( + import numpy as np + table_dict = {b'a': np.array([1] * 4, dtype='i8'), + b'b': np.array([2] * 4, dtype='f8')} + )"); + ASSERT_TRUE(ns); + + py::borrowed_ref table_dict = PyDict_GetItemString(ns.get(), "table_dict"); + ASSERT_TRUE(table_dict); + + using my_table_view = py::table_view("a"_cs), + py::C("b"_cs)>; + auto table_view = py::from_object(table_dict); + ASSERT_EQ(table_view.size(), 4ul); + + for (auto row_view : table_view) { + const auto& a = row_view.get("a"_cs); + const auto& b = row_view.get("b"_cs); + + EXPECT_EQ(a, 1); + EXPECT_EQ(b, 2.0); + } +} + TEST_F(from_object, object_map_key) { PyObject* ob = Py_None; Py_ssize_t starting_ref_count = Py_REFCNT(ob); From ba84f00ad445ead389ac5de0291f8ca4dc1ef101 Mon Sep 17 00:00:00 2001 From: Richard Frank Date: Thu, 21 May 2020 16:47:07 -0400 Subject: [PATCH 2/2] BLD: Link to stdc++fs on mac gcc<9 to prevent "Symbol not found" --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index f82f5b41..e5510902 100644 --- a/Makefile +++ b/Makefile @@ -77,6 +77,10 @@ ifeq ($(OS),Darwin) ifeq ($(COMPILER),GCC) # #sparseahash uses realloc which osx+gcc are not happy about CXXFLAGS += -Wno-class-memaccess + GCCVERSIONLT9 := $(shell test `$(COMPILER) -dumpversion | cut -f1 -d.` -lt 9 && echo true) +ifeq ($(GCCVERSIONLT9),true) + LDFLAGS += -lstdc++fs +endif endif else CXXFLAGS += -fstack-protector-strong