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

Detail: type naming convention #1016

Open
dean0x7d opened this issue Aug 21, 2017 · 3 comments
Open

Detail: type naming convention #1016

dean0x7d opened this issue Aug 21, 2017 · 3 comments

Comments

@dean0x7d
Copy link
Member

dean0x7d commented Aug 21, 2017

In the pybind11 detail code, we have 3 different kinds of type information:

  1. C++ RTTI: std::type_info
  2. Python's PyTypeObject
  3. pybind11's extended type information: detail::type_info

The variable naming scheme for these 3 kinds of type info objects isn't currently very consistent. type, tinfo, type_info, etc. are used interchangeably for all 3 kinds of type info, which can cause confusion (at least it does for me). And as I've recently found in #1014, it can result in some unfortunate expressions like x.type->type (where the second type isn't a Python-style metatype, it's just a different kind of type object).

I'd like to propose a naming convention for these objects (especially when they are members):

  1. std::type_info: variables named cpptype (already in use, but only partially).
  2. PyTypeObject: pytype.
  3. detail::type_info: it would be nice to rename the type to detail::extended_type (to avoid confusion with std::type_info) and name variables like ext_type or et.

What do you think? Does it make sense or am I just bikeshedding?

@jagerman
Copy link
Member

jagerman commented Aug 21, 2017

I'm pretty much used to the three distinctions, but can see it might be helpful for newcomers to better distinguish between the three.

I remember finding 3. the most confusing at first; the detail::type_info and std::type_info being entirely different things goes against our common use case of bringing an stl-duplicate into detail when we're really just backporting the stl type to an earlier standard. I don't know about the extended_type name though: it's not really extending but more about storing the binding data. To throw out some other suggestions: how about type_details, registered_type, type_bindings, type_glue or something along those lines? (type_glue, in particular, gives an immediately obvious, short, and fairly unique member/variable name).

@YannickJadoul
Copy link
Collaborator

Good point; I recently bumped into the same (gentle) confusion. Let's make sure this gets discussed for 2.7 :-)

@EricCousineau-TRI
Copy link
Collaborator

+1 to above points. I think this should be easy to rename, and add in deprecated type aliases if any of it was used in downstream applications.

@henryiii henryiii modified the milestones: v2.7, v2.8 Jul 16, 2021
@henryiii henryiii modified the milestones: v2.8, v2.9 Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants