Skip to content

docstring type hint incorrect for argument depending on class declaration order #2263

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
ixje opened this issue Jun 24, 2020 · 4 comments
Closed

Comments

@ixje
Copy link

ixje commented Jun 24, 2020

Issue description

The generated type-hint for the argument in the initialiser of class B (see below) is invalid and shows the internal namespace instead of the module. Output of main.py (again see below)

__init__(...)
    __init__(self: cmake_example.B, arg0: testns::A) -> None

Note that the code is functional (although useless). To get the correct type hints we have to define py::class_<A>(m, "A") before py::class_<B>(m, "B")

__init__(...)
    __init__(self: cmake_example.B, arg0: cmake_example.A) -> None

expected behaviour
I'd expected to see type hints resolved correctly, regardless of class declaration order.

Reproducible example code

cmakelists.txt

cmake_minimum_required(VERSION 2.8.12)
project(cmake_example)

set(PYTHON_EXECUTABLE ${CMAKE_CURRENT_SOURCE_DIR}/venv/bin/python3)

include(FetchContent)
FetchContent_Declare(
        pybind11
        GIT_REPOSITORY https://github.com/pybind/pybind11.git
        GIT_TAG        v2.5.0
)
FetchContent_MakeAvailable(pybind11)
pybind11_add_module(cmake_example src/main.cpp)

main.cpp

#include <pybind11/pybind11.h>

namespace py = pybind11;
namespace testns {
    class A {
        A() =default;
    };

    class B {
    public:
        B(A* a) {}
    };
};

using namespace testns;


PYBIND11_MODULE(cmake_example, m) {
    py::class_<B>(m, "B")
            .def(py::init([](A* a) {
                return B(a);
            }));

    py::class_<A>(m, "A")
            .def(py::init());
}

main.py

import cmake_example

if __name__ == '__main__':
    print(help(cmake_example.B.__init__))
@bstaletic
Copy link
Collaborator

I don't think this is possible to resolve. While in the def(py::init(...)), you have no idea if the user will ever bind A. Even if the user will do that, how do you know that testns::A will be cmake_example.A? Maybe the user does py::class_<A>(m, "Z"), in which case the correct docstring would contain cmake_example.Z. I also don't see a way to "channel back" the information about A bindings to B (the key word being "back").

@ixje
Copy link
Author

ixje commented Jul 7, 2020

That's too bad. I was hoping for some sort of second pass to be able to solve it.

@ixje ixje closed this as completed Jul 7, 2020
@YannickJadoul
Copy link
Collaborator

You could structure your bindings in two passes, though, if you know you potentially have issues like this:

PYBIND11_MODULE(cmake_example, m) {
    py::class_<A> class_A(m, "A");
    py::class_<B> class_B(m, "B");

    class_A.def(py::init());
    class_B.def(py::init([](A* a) {
                return B(a);
            }));
}

Some internal second-pass might be technically possible, but would probably cause a lot of rewriting and maybe an extra cost, and so is probably not worth it.

I do have some ugly system for automatically doing the two-pass bindings in my project, if you're interested: https://github.com/YannickJadoul/Parselmouth/blob/f899207fb60dbcfea6acd3d87d62d3be8ad62c1c/src/Bindings.h
There's a few problems with it (mostly that macros are involved) that I might wish to fix, but feel free to use this if it's useful to you.

@ixje
Copy link
Author

ixje commented Jul 7, 2020

Thanks for the tip @YannickJadoul !

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

No branches or pull requests

3 participants