Merge pull request #425 from dean0x7d/accessors

Make the accessor interface more complete

Merge pull request #425 from dean0x7d/accessors
Make the accessor interface more complete
0a014e91 · Wenzel Jakob · GitHub · 2d9220f0 · 2bab5793 · 0a014e91
Commit 0a014e91 authored Sep 26, 2016 by Wenzel Jakob Committed by GitHub Sep 26, 2016
10 changed files
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -53,6 +53,8 @@ Breaking changes queued for v2.0.0 (Not yet released)
 * Added ``py::dict`` keyword constructor:``auto d = dict("number"_a=42, "name"_a="World");``
 * Added ``py::str::format()`` method and ``_s`` literal:
  ``py::str s = "1 + 2 = {}"_s.format(3);``
+* Attribute and item accessors now have a more complete interface which makes it possible
+  to chain attributes ``obj.attr("a")[key].attr("b").attr("method")(1, 2, 3)```.
 * Various minor improvements of library internals (no user-visible changes)
 1.8.1 (July 12, 2016)

--- a/include/pybind11/attr.h
+++ b/include/pybind11/attr.h
@@ -276,7 +276,7 @@ template <> struct process_attribute<arg_v> : process_attribute_default<arg_v> {
 /// Process a parent class attribute
 template <typename T>
-struct process_attribute<T, enable_if_t<std::is_base_of<handle, T>::value>> : process_attribute_default<handle> {
+struct process_attribute<T, enable_if_t<is_pyobject<T>::value>> : process_attribute_default<handle> {
    static void init(const handle &h, type_record *r) { r->bases.append(h); }
 };

--- a/include/pybind11/cast.h
+++ b/include/pybind11/cast.h
@@ -39,7 +39,10 @@ PYBIND11_NOINLINE inline internals &get_internals() {
        return *internals_ptr;
    handle builtins(PyEval_GetBuiltins());
    const char *id = PYBIND11_INTERNALS_ID;
-    capsule caps(builtins[id]);
+    capsule caps;
+    if (builtins.contains(id)) {
+        caps = builtins[id];
+    }
    if (caps.check()) {
        internals_ptr = caps;
    } else {
@@ -908,7 +911,7 @@ template <> struct handle_type_name<args> { static PYBIND11_DESCR name() { retur
 template <> struct handle_type_name<kwargs> { static PYBIND11_DESCR name() { return _("**kwargs"); } };
 template <typename type>
-struct type_caster<type, enable_if_t<std::is_base_of<handle, type>::value>> {
+struct type_caster<type, enable_if_t<is_pyobject<type>::value>> {
 public:
    template <typename T = type, enable_if_t<!std::is_base_of<object, T>::value, int> = 0>
    bool load(handle src, bool /* convert */) { value = type(src); return value.check(); }
@@ -1216,12 +1219,12 @@ private:
    void process(list &args_list, detail::args_proxy ap) {
        for (const auto &a : ap) {
-            args_list.append(a.cast<object>());
+            args_list.append(a);
        }
    }
    void process(list &/*args_list*/, arg_v a) {
-        if (m_kwargs[a.name]) {
+        if (m_kwargs.contains(a.name)) {
 #if defined(NDEBUG)
            multiple_values_error();
 #else
@@ -1240,7 +1243,7 @@ private:
    void process(list &/*args_list*/, detail::kwargs_proxy kp) {
        for (const auto &k : dict(kp, true)) {
-            if (m_kwargs[k.first]) {
+            if (m_kwargs.contains(k.first)) {
 #if defined(NDEBUG)
                multiple_values_error();
 #else
@@ -1296,18 +1299,20 @@ unpacking_collector<policy> collect_arguments(Args &&...args) {
    return { std::forward<Args>(args)... };
 }
-NAMESPACE_END(detail)
+template <typename Derived>
 template <return_value_policy policy, typename... Args>
-object handle::operator()(Args &&...args) const {
+object object_api<Derived>::operator()(Args &&...args) const {
-    return detail::collect_arguments<policy>(std::forward<Args>(args)...).call(m_ptr);
+    return detail::collect_arguments<policy>(std::forward<Args>(args)...).call(derived().ptr());
 }
-template <return_value_policy policy,
+template <typename Derived>
-          typename... Args> object handle::call(Args &&... args) const {
+template <return_value_policy policy, typename... Args>
+object object_api<Derived>::call(Args &&...args) const {
    return operator()<policy>(std::forward<Args>(args)...);
 }
+NAMESPACE_END(detail)
 #define PYBIND11_MAKE_OPAQUE(Type) \
    namespace pybind11 { namespace detail { \
        template<> class type_caster<Type> : public type_caster_base<Type> { }; \

--- a/include/pybind11/numpy.h
+++ b/include/pybind11/numpy.h
@@ -125,11 +125,11 @@ private:
    static npy_api lookup() {
        module m = module::import("numpy.core.multiarray");
-        object c = (object) m.attr("_ARRAY_API");
+        auto c = m.attr("_ARRAY_API");
 #if PY_MAJOR_VERSION >= 3
-        void **api_ptr = (void **) (c ? PyCapsule_GetPointer(c.ptr(), NULL) : nullptr);
+        void **api_ptr = (void **) PyCapsule_GetPointer(c.ptr(), NULL);
 #else
-        void **api_ptr = (void **) (c ? PyCObject_AsVoidPtr(c.ptr()) : nullptr);
+        void **api_ptr = (void **) PyCObject_AsVoidPtr(c.ptr());
 #endif
        npy_api api;
 #define DECL_NPY_API(Func) api.Func##_ = (decltype(api.Func##_)) api_ptr[API_##Func];
@@ -220,9 +220,7 @@ private:
        struct field_descr { PYBIND11_STR_TYPE name; object format; pybind11::int_ offset; };
        std::vector<field_descr> field_descriptors;
-        auto fields = attr("fields").cast<object>();
+        for (auto field : attr("fields").attr("items")()) {
-        auto items = fields.attr("items").cast<object>();
-        for (auto field : items()) {
            auto spec = object(field, true).cast<tuple>();
            auto name = spec[0].cast<pybind11::str>();
            auto format = spec[1].cast<tuple>()[0].cast<dtype>();

--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -18,6 +18,7 @@
 #  pragma warning(disable: 4800) // warning C4800: 'int': forcing value to bool 'true' or 'false' (performance warning)
 #  pragma warning(disable: 4996) // warning C4996: The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name
 #  pragma warning(disable: 4702) // warning C4702: unreachable code
+#  pragma warning(disable: 4522) // warning C4522: multiple assignment operators specified
 #elif defined(__INTEL_COMPILER)
 #  pragma warning(push)
 #  pragma warning(disable: 186)   // pointless comparison of unsigned integer with zero
@@ -176,7 +177,7 @@ protected:
            if (a.descr)
                a.descr = strdup(a.descr);
            else if (a.value)
-                a.descr = strdup(((std::string) ((object) handle(a.value).attr("__repr__"))().str()).c_str());
+                a.descr = strdup(a.value.attr("__repr__")().cast<std::string>().c_str());
        }
        auto const &registered_types = detail::get_internals().registered_types_cpp;
@@ -278,9 +279,11 @@ protected:
            object scope_module;
            if (rec->scope) {
-                scope_module = (object) rec->scope.attr("__module__");
+                if (hasattr(rec->scope, "__module__")) {
-                if (!scope_module)
+                    scope_module = rec->scope.attr("__module__");
-                    scope_module = (object) rec->scope.attr("__name__");
+                } else if (hasattr(rec->scope, "__name__")) {
+                    scope_module = rec->scope.attr("__name__");
+                }
            }
            m_ptr = PyCFunction_NewEx(rec->def, rec_capsule.ptr(), scope_module.ptr());
@@ -544,8 +547,8 @@ public:
    template <typename Func, typename... Extra>
    module &def(const char *name_, Func &&f, const Extra& ... extra) {
-        cpp_function func(std::forward<Func>(f), name(name_),
+        cpp_function func(std::forward<Func>(f), name(name_), scope(*this),
-                          sibling((handle) attr(name_)), scope(*this), extra...);
+                          sibling(getattr(*this, name_, none())), extra...);
        /* PyModule_AddObject steals a reference to 'func' */
        PyModule_AddObject(ptr(), name_, func.inc_ref().ptr());
        return *this;
@@ -588,16 +591,18 @@ protected:
        object name(PYBIND11_FROM_STRING(rec->name), false);
        object scope_module;
        if (rec->scope) {
-            scope_module = (object) rec->scope.attr("__module__");
+            if (hasattr(rec->scope, "__module__")) {
-            if (!scope_module)
+                scope_module = rec->scope.attr("__module__");
-                scope_module = (object) rec->scope.attr("__name__");
+            } else if (hasattr(rec->scope, "__name__")) {
+                scope_module = rec->scope.attr("__name__");
+            }
        }
 #if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
        /* Qualified names for Python >= 3.3 */
        object scope_qualname;
-        if (rec->scope)
+        if (rec->scope && hasattr(rec->scope, "__qualname__"))
-            scope_qualname = (object) rec->scope.attr("__qualname__");
+            scope_qualname = rec->scope.attr("__qualname__");
        object ht_qualname;
        if (scope_qualname) {
            ht_qualname = object(PyUnicode_FromFormat(
@@ -719,8 +724,7 @@ protected:
        if (ob_type == &PyType_Type) {
            std::string name_ = std::string(ht_type.tp_name) + "__Meta";
 #if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
-            object ht_qualname(PyUnicode_FromFormat(
+            object ht_qualname(PyUnicode_FromFormat("%U__Meta", attr("__qualname__").ptr()), false);
-                "%U__Meta", ((object) attr("__qualname__")).ptr()), false);
 #endif
            object name(PYBIND11_FROM_STRING(name_.c_str()), false);
            object type_holder(PyType_Type.tp_alloc(&PyType_Type, 0), false);
@@ -894,17 +898,16 @@ public:
    template <typename Func, typename... Extra>
    class_ &def(const char *name_, Func&& f, const Extra&... extra) {
-        cpp_function cf(std::forward<Func>(f), name(name_),
+        cpp_function cf(std::forward<Func>(f), name(name_), is_method(*this),
-                        sibling(attr(name_)), is_method(*this),
+                        sibling(getattr(*this, name_, none())), extra...);
-                        extra...);
        attr(cf.name()) = cf;
        return *this;
    }
    template <typename Func, typename... Extra> class_ &
    def_static(const char *name_, Func f, const Extra&... extra) {
-        cpp_function cf(std::forward<Func>(f), name(name_),
+        cpp_function cf(std::forward<Func>(f), name(name_), scope(*this),
-                        sibling(attr(name_)), scope(*this), extra...);
+                        sibling(getattr(*this, name_, none())), extra...);
        attr(cf.name()) = cf;
        return *this;
    }
@@ -1336,19 +1339,19 @@ NAMESPACE_BEGIN(detail)
 PYBIND11_NOINLINE inline void print(tuple args, dict kwargs) {
    auto strings = tuple(args.size());
    for (size_t i = 0; i < args.size(); ++i) {
-        strings[i] = args[i].cast<object>().str();
+        strings[i] = args[i].str();
    }
-    auto sep = kwargs["sep"] ? kwargs["sep"] : cast(" ");
+    auto sep = kwargs.contains("sep") ? kwargs["sep"] : cast(" ");
-    auto line = sep.attr("join").cast<object>()(strings);
+    auto line = sep.attr("join")(strings);
-    auto file = kwargs["file"] ? kwargs["file"].cast<object>()
+    auto file = kwargs.contains("file") ? kwargs["file"].cast<object>()
                                        : module::import("sys").attr("stdout");
-    auto write = file.attr("write").cast<object>();
+    auto write = file.attr("write");
    write(line);
-    write(kwargs["end"] ? kwargs["end"] : cast("\n"));
+    write(kwargs.contains("end") ? kwargs["end"] : cast("\n"));
-    if (kwargs["flush"] && kwargs["flush"].cast<bool>()) {
+    if (kwargs.contains("flush") && kwargs["flush"].cast<bool>()) {
-        file.attr("flush").cast<object>()();
+        file.attr("flush")();
    }
 }
 NAMESPACE_END(detail)
@@ -1500,7 +1503,7 @@ inline function get_type_overload(const void *this_ptr, const detail::type_info 
    if (cache.find(key) != cache.end())
        return function();
-    function overload = (function) py_object.attr(name);
+    function overload = getattr(py_object, name, function());
    if (overload.is_cpp_function()) {
        cache.insert(key);
        return function();

--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
--- a/tests/constructor_stats.h
+++ b/tests/constructor_stats.h
@@ -103,7 +103,7 @@ public:
    int alive() {
        // Force garbage collection to ensure any pending destructors are invoked:
-        py::module::import("gc").attr("collect").operator py::object()();
+        py::module::import("gc").attr("collect")();
        int total = 0;
        for (const auto &p : _instances) if (p.second > 0) total += p.second;
        return total;

--- a/tests/pybind11_tests.cpp
+++ b/tests/pybind11_tests.cpp
@@ -39,7 +39,7 @@ PYBIND11_PLUGIN(pybind11_tests) {
    for (const auto &initializer : initializers())
        initializer(m);
-    if (!m.attr("have_eigen")) m.attr("have_eigen") = py::cast(false);
+    if (!py::hasattr(m, "have_eigen")) m.attr("have_eigen") = py::cast(false);
    return m.ptr();
 }
--- a/tests/test_python_types.cpp
+++ b/tests/test_python_types.cpp
@@ -60,7 +60,7 @@ public:
    py::list get_list() {
        py::list list;
        list.append(py::str("value"));
-        py::print("Entry at position 0:", py::object(list[0]));
+        py::print("Entry at position 0:", list[0]);
        list[0] = py::str("overwritten");
        return list;
    }
@@ -203,7 +203,7 @@ test_initializer python_types([](py::module &m) {
        py::print("no new line here", "end"_a=" -- ");
        py::print("next print");
-        auto py_stderr = py::module::import("sys").attr("stderr").cast<py::object>();
+        auto py_stderr = py::module::import("sys").attr("stderr");
        py::print("this goes to stderr", "file"_a=py_stderr);
        py::print("flush", "flush"_a=true);
@@ -222,4 +222,71 @@ test_initializer python_types([](py::module &m) {
        auto d2 = py::dict("z"_a=3, **d1);
        return d2;
    });
+    m.def("test_accessor_api", [](py::object o) {
+        auto d = py::dict();
+        d["basic_attr"] = o.attr("basic_attr");
+        auto l = py::list();
+        for (const auto &item : o.attr("begin_end")) {
+            l.append(item);
+        }
+        d["begin_end"] = l;
+        d["operator[object]"] = o.attr("d")["operator[object]"_s];
+        d["operator[char *]"] = o.attr("d")["operator[char *]"];
+        d["attr(object)"] = o.attr("sub").attr("attr_obj");
+        d["attr(char *)"] = o.attr("sub").attr("attr_char");
+        try {
+            o.attr("sub").attr("missing").ptr();
+        } catch (const py::error_already_set &) {
+            d["missing_attr_ptr"] = "raised"_s;
+        }
+        try {
+            o.attr("missing").attr("doesn't matter");
+        } catch (const py::error_already_set &) {
+            d["missing_attr_chain"] = "raised"_s;
+        }
+        d["is_none"] = py::cast(o.attr("basic_attr").is_none());
+        d["operator()"] = o.attr("func")(1);
+        d["operator*"] = o.attr("func")(*o.attr("begin_end"));
+        return d;
+    });
+    m.def("test_tuple_accessor", [](py::tuple existing_t) {
+        try {
+            existing_t[0] = py::cast(1);
+        } catch (const py::error_already_set &) {
+            // --> Python system error
+            // Only new tuples (refcount == 1) are mutable
+            auto new_t = py::tuple(3);
+            for (size_t i = 0; i < new_t.size(); ++i) {
+                new_t[i] = py::cast(i);
+            }
+            return new_t;
+        }
+        return py::tuple();
+    });
+    m.def("test_accessor_assignment", []() {
+        auto l = py::list(1);
+        l[0] = py::cast(0);
+        auto d = py::dict();
+        d["get"] = l[0];
+        auto var = l[0];
+        d["deferred_get"] = var;
+        l[0] = py::cast(1);
+        d["set"] = l[0];
+        var = py::cast(99); // this assignment should not overwrite l[0]
+        d["deferred_set"] = l[0];
+        d["var"] = var;
+        return d;
+    });
 });
--- a/tests/test_python_types.py
+++ b/tests/test_python_types.py
@@ -248,3 +248,42 @@ def test_dict_api():
    from pybind11_tests import test_dict_keyword_constructor
    assert test_dict_keyword_constructor() == {"x": 1, "y": 2, "z": 3}
+def test_accessors():
+    from pybind11_tests import test_accessor_api, test_tuple_accessor, test_accessor_assignment
+    class SubTestObject:
+        attr_obj = 1
+        attr_char = 2
+    class TestObject:
+        basic_attr = 1
+        begin_end = [1, 2, 3]
+        d = {"operator[object]": 1, "operator[char *]": 2}
+        sub = SubTestObject()
+        def func(self, x, *args):
+            return self.basic_attr + x + sum(args)
+    d = test_accessor_api(TestObject())
+    assert d["basic_attr"] == 1
+    assert d["begin_end"] == [1, 2, 3]
+    assert d["operator[object]"] == 1
+    assert d["operator[char *]"] == 2
+    assert d["attr(object)"] == 1
+    assert d["attr(char *)"] == 2
+    assert d["missing_attr_ptr"] == "raised"
+    assert d["missing_attr_chain"] == "raised"
+    assert d["is_none"] is False
+    assert d["operator()"] == 2
+    assert d["operator*"] == 7
+    assert test_tuple_accessor(tuple()) == (0, 1, 2)
+    d = test_accessor_assignment()
+    assert d["get"] == 0
+    assert d["deferred_get"] == 0
+    assert d["set"] == 1
+    assert d["deferred_set"] == 1
+    assert d["var"] == 99