From 384f4ada1f3f08486fb03427227878ddbbcaad43 Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Fri, 14 Oct 2022 21:05:43 +0200 Subject: [PATCH] fixed function-level documentation in pydoc commit 0b3548ad6faf4e changed function objects in a subtle way that broke docstring extraction in pydoc. While the nb_func.__doc__ member worked as expected, pydoc no longer queried it. This commit both fixes the regression and adds tests to ensure that it doesn't happen again in the future (by running pydoc and looking for certain strings that we would expect to be part of the result). --- src/nb_func.cpp | 124 +++++++++++++++++++++------------------- src/nb_internals.cpp | 17 +++++- tests/test_classes.py | 5 ++ tests/test_functions.py | 5 ++ 4 files changed, 89 insertions(+), 62 deletions(-) diff --git a/src/nb_func.cpp b/src/nb_func.cpp index 85e1fada..ff65dbc3 100644 --- a/src/nb_func.cpp +++ b/src/nb_func.cpp @@ -962,81 +962,87 @@ static void nb_func_render_signature(const func_data *f) noexcept { fail("nanobind::detail::nb_func_finalize(%s): arguments inconsistent.", f->name); } -PyObject *nb_func_getattro(PyObject *self, PyObject *name_) { +PyObject *nb_func_get_name(PyObject *self, void *) { func_data *f = nb_func_data(self); - const char *name = PyUnicode_AsUTF8AndSize(name_, nullptr); - - if (!name) - return nullptr; + if (f->flags & (uint32_t) func_flags::has_name) { + return PyUnicode_FromString(f->name); + } else { + Py_INCREF(Py_None); + return Py_None; + } +} - if (strcmp(name, "__name__") == 0) { - if (f->flags & (uint32_t) func_flags::has_name) { +PyObject *nb_func_get_qualname(PyObject *self, void *) { + func_data *f = nb_func_data(self); + if ((f->flags & (uint32_t) func_flags::has_scope) && + (f->flags & (uint32_t) func_flags::has_name)) { + PyObject *scope_name = PyObject_GetAttrString(f->scope, "__qualname__"); + if (scope_name) { + return PyUnicode_FromFormat("%U.%s", scope_name, f->name); + } else { + PyErr_Clear(); return PyUnicode_FromString(f->name); } - } else if (strcmp(name, "__qualname__") == 0) { - if ((f->flags & (uint32_t) func_flags::has_scope) && - (f->flags & (uint32_t) func_flags::has_name)) { - PyObject *scope_name = PyObject_GetAttrString(f->scope, "__qualname__"); - if (scope_name) { - return PyUnicode_FromFormat("%U.%s", scope_name, f->name); - } else { - PyErr_Clear(); - return PyUnicode_FromString(f->name); - } - } - } else if (strcmp(name, "__module__") == 0) { - if (f->flags & (uint32_t) func_flags::has_scope) { - return PyObject_GetAttrString( - f->scope, PyModule_Check(f->scope) ? "__name__" : "__module__"); - } - } else if (strcmp(name, "__doc__") == 0) { - uint32_t count = (uint32_t) Py_SIZE(self); + } else { + Py_INCREF(Py_None); + return Py_None; + } +} + +PyObject *nb_func_get_module(PyObject *self, void *) { + func_data *f = nb_func_data(self); + if (f->flags & (uint32_t) func_flags::has_scope) { + return PyObject_GetAttrString( + f->scope, PyModule_Check(f->scope) ? "__name__" : "__module__"); + } else { + Py_INCREF(Py_None); + return Py_None; + } +} - buf.clear(); +PyObject *nb_func_get_doc(PyObject *self, void *) { + func_data *f = nb_func_data(self); + uint32_t count = (uint32_t) Py_SIZE(self); - size_t doc_count = 0; - for (uint32_t i = 0; i < count; ++i) { - const func_data *fi = f + i; - if (fi->flags & (uint32_t) func_flags::raw_doc) - return PyUnicode_FromString(fi->doc); + buf.clear(); - nb_func_render_signature(fi); - buf.put('\n'); - if ((fi->flags & (uint32_t) func_flags::has_doc) && fi->doc[0] != '\0') - doc_count++; - } + size_t doc_count = 0; + for (uint32_t i = 0; i < count; ++i) { + const func_data *fi = f + i; + if (fi->flags & (uint32_t) func_flags::raw_doc) + return PyUnicode_FromString(fi->doc); - if (doc_count > 1) - buf.put("\nOverloaded function.\n"); + nb_func_render_signature(fi); + buf.put('\n'); + if ((fi->flags & (uint32_t) func_flags::has_doc) && fi->doc[0] != '\0') + doc_count++; + } - for (uint32_t i = 0; i < count; ++i) { - const func_data *fi = f + i; + if (doc_count > 1) + buf.put("\nOverloaded function.\n"); - if ((fi->flags & (uint32_t) func_flags::has_doc) && fi->doc[0] != '\0') { - buf.put('\n'); + for (uint32_t i = 0; i < count; ++i) { + const func_data *fi = f + i; - if (doc_count > 1) { - buf.put_uint32(i + 1); - buf.put(". ``"); - nb_func_render_signature(fi); - buf.put("``\n\n"); - } + if ((fi->flags & (uint32_t) func_flags::has_doc) && fi->doc[0] != '\0') { + buf.put('\n'); - buf.put_dstr(fi->doc); - buf.put('\n'); + if (doc_count > 1) { + buf.put_uint32(i + 1); + buf.put(". ``"); + nb_func_render_signature(fi); + buf.put("``\n\n"); } - } - - if (buf.size() > 0) // remove last newline - buf.rewind(1); - return PyUnicode_FromString(buf.get()); - } else { - return PyObject_GenericGetAttr(self, name_); + buf.put_dstr(fi->doc); + buf.put('\n'); + } } - Py_INCREF(Py_None); - return Py_None; + if (buf.size() > 0) // remove last newline + buf.rewind(1); + + return PyUnicode_FromString(buf.get()); } /// Excise a substring from 's' diff --git a/src/nb_internals.cpp b/src/nb_internals.cpp index 58912918..f951061f 100644 --- a/src/nb_internals.cpp +++ b/src/nb_internals.cpp @@ -74,13 +74,16 @@ NAMESPACE_BEGIN(NB_NAMESPACE) NAMESPACE_BEGIN(detail) +extern PyObject *nb_func_get_doc(PyObject *, void *); +extern PyObject *nb_func_get_name(PyObject *, void *); +extern PyObject *nb_func_get_qualname(PyObject *, void *); +extern PyObject *nb_func_get_module(PyObject *, void *); extern int nb_func_traverse(PyObject *, visitproc, void *); extern int nb_func_clear(PyObject *); extern void nb_func_dealloc(PyObject *); extern int nb_bound_method_traverse(PyObject *, visitproc, void *); extern int nb_bound_method_clear(PyObject *); extern void nb_bound_method_dealloc(PyObject *); -extern PyObject *nb_func_getattro(PyObject *, PyObject *); extern PyObject *nb_method_descr_get(PyObject *, PyObject *, PyObject *); extern int nb_type_setattro(PyObject*, PyObject*, PyObject*); extern PyObject *nb_tensor_get(PyObject *, PyObject *); @@ -102,12 +105,20 @@ static PyMemberDef nb_func_members[] = { { nullptr, 0, 0, 0, nullptr } }; +static PyGetSetDef nb_func_getset[] = { + { "__doc__", nb_func_get_doc, nullptr, nullptr, nullptr }, + { "__name__", nb_func_get_name, nullptr, nullptr, nullptr }, + { "__qualname__", nb_func_get_qualname, nullptr, nullptr, nullptr }, + { "__module__", nb_func_get_module, nullptr, nullptr, nullptr }, + { nullptr, nullptr, nullptr, nullptr, nullptr } +}; + static PyType_Slot nb_func_slots[] = { { Py_tp_members, (void *) nb_func_members }, + { Py_tp_getset, (void *) nb_func_getset }, { Py_tp_traverse, (void *) nb_func_traverse }, { Py_tp_clear, (void *) nb_func_clear }, { Py_tp_dealloc, (void *) nb_func_dealloc }, - { Py_tp_getattro, (void *) nb_func_getattro }, { Py_tp_traverse, (void *) nb_func_traverse}, { Py_tp_new, (void *) PyType_GenericNew }, { Py_tp_call, (void *) PyVectorcall_Call }, @@ -124,10 +135,10 @@ static PyType_Spec nb_func_spec = { static PyType_Slot nb_method_slots[] = { { Py_tp_members, (void *) nb_func_members }, + { Py_tp_getset, (void *) nb_func_getset }, { Py_tp_traverse, (void *) nb_func_traverse }, { Py_tp_clear, (void *) nb_func_clear }, { Py_tp_dealloc, (void *) nb_func_dealloc }, - { Py_tp_getattro, (void *) nb_func_getattro }, { Py_tp_descr_get, (void *) nb_method_descr_get }, { Py_tp_new, (void *) PyType_GenericNew }, { Py_tp_call, (void *) PyVectorcall_Call }, diff --git a/tests/test_classes.py b/tests/test_classes.py index 765bdcc0..361948f8 100644 --- a/tests/test_classes.py +++ b/tests/test_classes.py @@ -505,3 +505,8 @@ def test27_copy_rvp(): a = t.Struct.create_reference() b = t.Struct.create_copy() assert a is not b + + +def test28_pydoc(): + import pydoc + assert "Some documentation" in pydoc.render_doc(t) diff --git a/tests/test_functions.py b/tests/test_functions.py index 8d0839b9..500d530e 100644 --- a/tests/test_functions.py +++ b/tests/test_functions.py @@ -203,3 +203,8 @@ def test23_byte_return(): assert t.test_17(b"four") == 4 assert t.test_17(b"\x00\x00\x00\x00") == 4 assert t.test_18("hello world", 5) == b"hello" + + +def test24_pydoc(): + import pydoc + assert "test_05(arg: int, /)" in pydoc.render_doc(t)