Merge pull request #151 from quantopian/table-from-object

BUG: Update from_object dispatch for table

Author: richafrank

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

include/libpy/table.h

@@ -1140,39 +1140,39 @@ struct from_object<py::table_view<columns...>> {
     using type = py::table_view<columns...>;
 
     template<typename Column>
-    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<std::array<char, text.size() - 1>*>(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();
         }
         return py::from_object<py::array_view<typename Column::value>>(column_ob);
     }
 
 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<table_view<...>> 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<py::detail::unwrap_column<columns>>(copy.get())...);
+        type out(pop_column<py::detail::unwrap_column<columns>>(copy)...);
         if (PyDict_Size(copy.get())) {
             py::owned_ref keys(PyDict_Keys(copy.get()));
             if (!keys) {
@@ -1190,39 +1190,39 @@ struct from_object<py::row<columns...>> {
     using type = py::row<columns...>;
 
     template<typename Column>
-    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<std::array<char, text.size() - 1>*>(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();
         }
         return py::from_object<typename Column::value>(column_ob);
     }
 
 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<table_view<...>> input must be a Python dictionary, got: ",
-                Py_TYPE(t)->tp_name);
+                "from_object<row<...>> 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<py::detail::unwrap_column<columns>>(copy.get())...);
+        type out(pop_column<py::detail::unwrap_column<columns>>(copy)...);
         if (PyDict_Size(copy.get())) {
             py::owned_ref keys(PyDict_Keys(copy.get()));
             if (!keys) {

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<typename T>
@@ -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<py::C<std::int64_t>("a"_cs),
+                                         py::C<double>("b"_cs)>;
+    auto table_view = py::from_object<my_table_view>(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);