Skip to content

Incorrect type names in docstrings for nested types #1166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jondreads opened this issue Nov 3, 2017 · 4 comments · Fixed by #1171
Closed

Incorrect type names in docstrings for nested types #1166

jondreads opened this issue Nov 3, 2017 · 4 comments · Fixed by #1171
Assignees

Comments

@jondreads
Copy link

Issue description

I have an inner enum. The type name for the enum in docstrings is not correct. I'm not sure if this is a bug in docstring generation, but I can't see any documentation on how to supply the correct name.

Reproducible example code

#include <pybind11/pybind11.h>
namespace py = pybind11;
struct Pet
{
  enum class Kind { Dog };
  Pet() : type(Kind::Dog) {}
  Kind type;
};
void foo(Pet::Kind) {}
PYBIND11_MODULE (hello, m)
{
  py::class_<Pet> pet(m, "Pet");
  pet.def(py::init())
    .def_readwrite("type", &Pet::type);
  py::enum_<Pet::Kind>(pet, "Kind")
    .value("Dog", Pet::Kind::Dog);
  m.def("foo", &foo);
}

In python

>>> import hello
>>> hello.foo.__doc__
'foo(arg0: hello.Kind) -> None\n'
>>> help(hello.Pet.Kind)
Help on class Kind in module hello:
class Kind(pybind11_builtins.pybind11_object)
...
 |  __eq__(...)
 |      __eq__(self: hello.Kind, arg0: hello.Kind) -> bool
...

I would expect to see "hello.Pet.Kind" in the docstrings not "hello.Kind"

@wjakob
Copy link
Member

wjakob commented Nov 3, 2017

Which version of Python are you using? There are some differences here between 2.7 and 3.x

@jondreads
Copy link
Author

jondreads commented Nov 3, 2017 via email

@jagerman
Copy link
Member

jagerman commented Nov 3, 2017

Some thoughts:

For Python 3.3+, this is something we can (and should) address. It isn't specific to enums, though, but rather classes (this shows up for enum_ because it inherits from class_).

The fix is for us to use __qualname__ rather than tp_name when displaying the type in the docstring; that should get us the nested name, e.g. Pet.Kind.

That's an easy enough change, except it exposed something else that doesn't look right: our __qualname__ values contain the module name, which they shouldn't.

E.g. a Python module pymod.py:

def foo():
    pass

class C():
    pass

Python:

Python 3.6.3 (default, Oct  3 2017, 21:16:13) 
[GCC 7.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pymod
>>> (pymod.foo.__name__, pymod.foo.__module__, pymod.foo.__qualname__)
('foo', 'pymod', 'foo')
>>> (pymod.C.__name__, pymod.C.__module__, pymod.C.__qualname__)
('C', 'pymod', 'C')
>>> (pymod.C.D.__name__, pymod.C.D.__module__, pymod.C.D.__qualname__)
('D', 'pymod', 'C.D')

while the equivalent using pybind11 is incorrectly tacking the module name into the __qualname__:

('D', 'mod', 'mod.C.D')

So this is going to take a bit more than just printing __qualname__ instead of tp_name.

@jagerman jagerman changed the title Incorrect type names in docstrings for inner enums Incorrect type names in docstrings for nested types Nov 3, 2017
@jagerman jagerman self-assigned this Nov 3, 2017
jagerman added a commit to jagerman/pybind11 that referenced this issue Nov 6, 2017
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 pybind#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.
jagerman added a commit that referenced this issue Nov 7, 2017
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.
@jagerman
Copy link
Member

jagerman commented Nov 7, 2017

This should be fixed now on master via #1171.

jagerman added a commit to jagerman/pybind11 that referenced this issue Jan 12, 2018
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 pybind#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.
wjakob pushed a commit that referenced this issue Feb 7, 2018
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants