Skip to content

Commit

Permalink
fixed function-level documentation in pydoc
Browse files Browse the repository at this point in the history 
commit 0b3548a 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).
  • Loading branch information
@wjakob
wjakob committed Oct 14, 2022
1 parent a2b8b20 commit 384f4ad
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 62 deletions.
124 changes: 65 additions & 59 deletions src/nb_func.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
17 changes: 14 additions & 3 deletions src/nb_internals.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);
Expand All @@ -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 },
Expand All @@ -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 },
Expand Down
5 changes: 5 additions & 0 deletions tests/test_classes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
5 changes: 5 additions & 0 deletions tests/test_functions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

0 comments on commit 384f4ad

Please sign in to comment.