Skip to content
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

Fix pydoc failure because of missing __module__ #729

Merged
merged 1 commit into from
Mar 15, 2017

Conversation

dean0x7d
Copy link
Member

Fixes #728.

The __module__ for all pybind11 types is set to pybind11_builtins, analogous to Python's own builtins.

@wjakob
Copy link
Member

wjakob commented Mar 15, 2017

Looks good to me!

@dean0x7d dean0x7d merged commit 1769ea4 into pybind:master Mar 15, 2017
@wojdyr
Copy link
Contributor

wojdyr commented Mar 15, 2017

Maybe a shorter name for pybind11_builtins.pybind11_object_*?
Pydoc output feels a bit cluttered. For example the example from http://pybind11.readthedocs.io/en/master/classes.html#overloaded-methods is now:

class Pet(pybind11_builtins.pybind11_object_48)
 |  Method resolution order:
 |      Pet
 |      pybind11_builtins.pybind11_object_48
 |      __builtin__.object
 |  
 |  Methods defined here:
 |  
 |  __init__(...)
 |      __init__(self: example.Pet, arg0: unicode, arg1: int) -> None
 |  
 |  set(...)
 |      set(*args, **kwargs)
 |      Overloaded function.
 |      
 |      1. set(self: example.Pet, arg0: int) -> None
 |      
 |      Set the pet's age
 |      
 |      2. set(self: example.Pet, arg0: unicode) -> None
 |      
 |      Set the pet's name
 |  
 |  ----------------------------------------------------------------------
 |  Data and other attributes inherited from pybind11_builtins.pybind11_object_48:
 |  
 |  __new__ = <built-in method __new__ of pybind11_type object>
 |      T.__new__(S, ...) -> a new object with type S, a subtype of T

@jagerman
Copy link
Member

I suppose it could be just pybind11_builtins.object_48.

It might also be nice to drop the "_48" for the default size (i.e. for the size used for a typical unique_ptr holder). So then we'd have pybind11_builtins.object typically, but the size suffix (e.g. pybind11_builtins.object_56) for other types (e.g. a type with a shared_ptr holder).

In 2.2 (with PR #693) the size suffix should be gone regardless of the holder.

@dean0x7d
Copy link
Member Author

I think it would be nice to avoid shadowing builtin names like object and type. While they are formally in the __builtin__ (2.7) / builtins (3.x) modules, the objects are always available unqualified globally. I don't think that shadowing them would be too much of a technical issue, but perhaps confusing upon user inspection, especially as Python 2.7 uses unqualified names. E.g. looking up type(A).__name__ for a pybind11 class and seeing type just like a regular Python class may lead someone to the wrong conclusion while debugging. Similarly for bases and object.

That said, I'm not attached to the pybind11_-prefix-everywhere naming scheme and don't mind a bit of bikeshedding on this, just without shadowing the builtins. Note that __module__ == "" is a possibility, I just didn't go for it here because pydoc leaves the dot prefix anyway.

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 this pull request may close these issues.

4 participants