From 71178922fdf375f1b58724a1c67fe11abc52e815 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Tue, 7 Nov 2017 12:33:05 -0400 Subject: [PATCH] __qualname__ and nested class naming fixes (#1171) A few fixes related to how we set `__qualname__` and how we show the type name in function signatures: - `__qualname__` isn't supposed to have the module name at the beginning, but we've been putting it there. This removes it, while keeping the `Nested.Class` name chaining. - print `__module__.__qualname__` rather than `type->tp_name`; the latter doesn't work properly for nested classes, so we would get `module.B` rather than `module.A.B` for a class `B` with parent `A`. This also unifies the Python 3 and PyPy code. Fixes #1166. - This now sets a `__qualname__` attribute on the type (as would happen in Python 3.3+) for Python <3.3, including PyPy. While not particularly important to have in earlier Python versions, it's useful for us to be able to extracted the nested name, which is why `__qualname__` was invented in the first place. - Added tests for the above. --- include/pybind11/detail/class.h | 36 ++++++++++++++++++++++++--------- include/pybind11/pybind11.h | 17 +++++++--------- tests/test_class.cpp | 15 ++++++++++++++ tests/test_class.py | 33 +++++++++++++++++++++++++++--- 4 files changed, 78 insertions(+), 23 deletions(-) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index f745992a0..7a5dd0130 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -14,6 +14,15 @@ NAMESPACE_BEGIN(PYBIND11_NAMESPACE) NAMESPACE_BEGIN(detail) +#if PY_VERSION_HEX >= 0x03030000 +# define PYBIND11_BUILTIN_QUALNAME +# define PYBIND11_SET_OLDPY_QUALNAME(obj, nameobj) +#else +// In pre-3.3 Python, we still set __qualname__ so that we can produce reliable function type +// signatures; in 3.3+ this macro expands to nothing: +# define PYBIND11_SET_OLDPY_QUALNAME(obj, nameobj) setattr((PyObject *) obj, "__qualname__", nameobj) +#endif + inline PyTypeObject *type_incref(PyTypeObject *type) { Py_INCREF(type); return type; @@ -48,7 +57,7 @@ inline PyTypeObject *make_static_property_type() { pybind11_fail("make_static_property_type(): error allocating type!"); heap_type->ht_name = name_obj.inc_ref().ptr(); -#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 +#ifdef PYBIND11_BUILTIN_QUALNAME heap_type->ht_qualname = name_obj.inc_ref().ptr(); #endif @@ -63,6 +72,7 @@ inline PyTypeObject *make_static_property_type() { pybind11_fail("make_static_property_type(): failure in PyType_Ready()!"); setattr((PyObject *) type, "__module__", str("pybind11_builtins")); + PYBIND11_SET_OLDPY_QUALNAME(type, name_obj); return type; } @@ -161,7 +171,7 @@ inline PyTypeObject* make_default_metaclass() { pybind11_fail("make_default_metaclass(): error allocating metaclass!"); heap_type->ht_name = name_obj.inc_ref().ptr(); -#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 +#ifdef PYBIND11_BUILTIN_QUALNAME heap_type->ht_qualname = name_obj.inc_ref().ptr(); #endif @@ -179,6 +189,7 @@ inline PyTypeObject* make_default_metaclass() { pybind11_fail("make_default_metaclass(): failure in PyType_Ready()!"); setattr((PyObject *) type, "__module__", str("pybind11_builtins")); + PYBIND11_SET_OLDPY_QUALNAME(type, name_obj); return type; } @@ -363,7 +374,7 @@ inline PyObject *make_object_base_type(PyTypeObject *metaclass) { pybind11_fail("make_object_base_type(): error allocating type!"); heap_type->ht_name = name_obj.inc_ref().ptr(); -#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 +#ifdef PYBIND11_BUILTIN_QUALNAME heap_type->ht_qualname = name_obj.inc_ref().ptr(); #endif @@ -384,6 +395,7 @@ inline PyObject *make_object_base_type(PyTypeObject *metaclass) { pybind11_fail("PyType_Ready failed in make_object_base_type():" + error_string()); setattr((PyObject *) type, "__module__", str("pybind11_builtins")); + PYBIND11_SET_OLDPY_QUALNAME(type, name_obj); assert(!PyType_HasFeature(type, Py_TPFLAGS_HAVE_GC)); return (PyObject *) heap_type; @@ -504,13 +516,15 @@ inline void enable_buffer_protocol(PyHeapTypeObject *heap_type) { inline PyObject* make_new_python_type(const type_record &rec) { auto name = reinterpret_steal(PYBIND11_FROM_STRING(rec.name)); -#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 - auto ht_qualname = name; - if (rec.scope && hasattr(rec.scope, "__qualname__")) { - ht_qualname = reinterpret_steal( + auto qualname = name; + if (rec.scope && !PyModule_Check(rec.scope.ptr()) && hasattr(rec.scope, "__qualname__")) { +#if PY_MAJOR_VERSION >= 3 + qualname = reinterpret_steal( PyUnicode_FromFormat("%U.%U", rec.scope.attr("__qualname__").ptr(), name.ptr())); - } +#else + qualname = str(rec.scope.attr("__qualname__").cast() + "." + rec.name); #endif + } object module; if (rec.scope) { @@ -552,8 +566,8 @@ inline PyObject* make_new_python_type(const type_record &rec) { pybind11_fail(std::string(rec.name) + ": Unable to create type object!"); heap_type->ht_name = name.release().ptr(); -#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3 - heap_type->ht_qualname = ht_qualname.release().ptr(); +#ifdef PYBIND11_BUILTIN_QUALNAME + heap_type->ht_qualname = qualname.inc_ref().ptr(); #endif auto type = &heap_type->ht_type; @@ -599,6 +613,8 @@ inline PyObject* make_new_python_type(const type_record &rec) { if (module) // Needed by pydoc setattr((PyObject *) type, "__module__", module); + PYBIND11_SET_OLDPY_QUALNAME(type, qualname); + return (PyObject *) type; } diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 90b8ebcc6..30c691702 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -246,19 +246,16 @@ protected: if (!t) pybind11_fail("Internal error while parsing type signature (1)"); if (auto tinfo = detail::get_type_info(*t)) { -#if defined(PYPY_VERSION) - signature += handle((PyObject *) tinfo->type) - .attr("__module__") - .cast() + "."; -#endif - signature += tinfo->type->tp_name; + handle th((PyObject *) tinfo->type); + signature += + th.attr("__module__").cast() + "." + + th.attr("__qualname__").cast(); // Python 3.3+, but we backport it to earlier versions } else if (rec->is_new_style_constructor && arg_index == 0) { // A new-style `__init__` takes `self` as `value_and_holder`. // Rewrite it to the proper class type. -#if defined(PYPY_VERSION) - signature += rec->scope.attr("__module__").cast() + "."; -#endif - signature += ((PyTypeObject *) rec->scope.ptr())->tp_name; + signature += + rec->scope.attr("__module__").cast() + "." + + rec->scope.attr("__qualname__").cast(); } else { std::string tname(t->name()); detail::clean_type_id(tname); diff --git a/tests/test_class.cpp b/tests/test_class.cpp index 222190617..927adf3f9 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -302,6 +302,21 @@ TEST_SUBMODULE(class_, m) { .def(py::init()); py::implicitly_convertible(); + + // test_qualname + // #1166: nested class docstring doesn't show nested name + // Also related: tests that __qualname__ is set properly + struct NestBase {}; + struct Nested {}; + py::class_ base(m, "NestBase"); + base.def(py::init<>()); + py::class_(base, "Nested") + .def(py::init<>()) + .def("fn", [](Nested &, int, NestBase &, Nested &) {}) + .def("fa", [](Nested &, int, NestBase &, Nested &) {}, + "a"_a, "b"_a, "c"_a); + base.def("g", [](NestBase &, Nested &) {}); + base.def("h", []() { return NestBase(); }); } template class BreaksBase { public: virtual ~BreaksBase() = default; }; diff --git a/tests/test_class.py b/tests/test_class.py index 412d6798e..d94b61bb3 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -44,6 +44,31 @@ def test_docstrings(doc): """ +def test_qualname(doc): + """Tests that a properly qualified name is set in __qualname__ (even in pre-3.3, where we + backport the attribute) and that generated docstrings properly use it and the module name""" + assert m.NestBase.__qualname__ == "NestBase" + assert m.NestBase.Nested.__qualname__ == "NestBase.Nested" + + assert doc(m.NestBase.__init__) == """ + __init__(self: m.class_.NestBase) -> None + """ + assert doc(m.NestBase.g) == """ + g(self: m.class_.NestBase, arg0: m.class_.NestBase.Nested) -> None + """ + assert doc(m.NestBase.Nested.__init__) == """ + __init__(self: m.class_.NestBase.Nested) -> None + """ + assert doc(m.NestBase.Nested.fn) == """ + fn(self: m.class_.NestBase.Nested, arg0: int, arg1: m.class_.NestBase, arg2: m.class_.NestBase.Nested) -> None + """ # noqa: E501 line too long + assert doc(m.NestBase.Nested.fa) == """ + fa(self: m.class_.NestBase.Nested, a: int, b: m.class_.NestBase, c: m.class_.NestBase.Nested) -> None + """ # noqa: E501 line too long + assert m.NestBase.__module__ == "pybind11_tests.class_" + assert m.NestBase.Nested.__module__ == "pybind11_tests.class_" + + def test_inheritance(msg): roger = m.Rabbit('Rabbit') assert roger.name() + " is a " + roger.species() == "Rabbit is a parrot" @@ -229,7 +254,9 @@ def test_reentrant_implicit_conversion_failure(msg): # ensure that there is no runaway reentrant implicit conversion (#1035) with pytest.raises(TypeError) as excinfo: m.BogusImplicitConversion(0) - assert msg(excinfo.value) == '''__init__(): incompatible constructor arguments. The following argument types are supported: - 1. m.class_.BogusImplicitConversion(arg0: m.class_.BogusImplicitConversion) + assert msg(excinfo.value) == ''' + __init__(): incompatible constructor arguments. The following argument types are supported: + 1. m.class_.BogusImplicitConversion(arg0: m.class_.BogusImplicitConversion) -Invoked with: 0''' + Invoked with: 0 + '''