Skip to content

Operator methods include a type signature of the additional overload #2277

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Operator methods include a type signature of the additional overload #2277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions include/pybind11/pybind11.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ class cpp_function : public function {
/* Generate a proper function signature */
std::string signature;
size_t type_index = 0, arg_index = 0;
std::string self_type("object");

This comment was marked as outdated.

Sign in to view

for (auto *pc = text; *pc != '\0'; ++pc) {
const auto c = *pc;

Expand Down Expand Up @@ -293,9 +294,13 @@ class cpp_function : public function {
pybind11_fail("Internal error while parsing type signature (1)");
if (auto tinfo = detail::get_type_info(*t)) {
handle th((PyObject *) tinfo->type);
signature +=
std::string tname =
th.attr("__module__").cast<std::string>() + "." +
th.attr("__qualname__").cast<std::string>(); // Python 3.3+, but we backport it to earlier versions
signature += tname;
if (arg_index == 0 && rec->is_method) {
self_type = tname;
}
} else if (rec->is_new_style_constructor && arg_index == 0) {
// A new-style `__init__` takes `self` as `value_and_holder`.
// Rewrite it to the proper class type.
Expand All @@ -306,6 +311,9 @@ class cpp_function : public function {
std::string tname(t->name());
detail::clean_type_id(tname);
signature += tname;
if (arg_index == 0 && rec->is_method) {
self_type = tname;
}
}
} else {
signature += c;
Expand Down Expand Up @@ -404,9 +412,10 @@ class cpp_function : public function {

std::string signatures;
int index = 0;
const bool has_overloads = (chain || rec->is_operator);
/* Create a nice pydoc rec including all signatures and
docstrings of the functions in the overload chain */
if (chain && options::show_function_signatures()) {
if (has_overloads && options::show_function_signatures()) {
// First a generic signature
signatures += rec->name;
signatures += "(*args, **kwargs)\n";
Expand All @@ -417,7 +426,7 @@ class cpp_function : public function {
for (auto it = chain_start; it != nullptr; it = it->next) {
if (options::show_function_signatures()) {
if (index > 0) signatures += "\n";
if (chain)
if (has_overloads)
signatures += std::to_string(++index) + ". ";
signatures += rec->name;
signatures += it->signature;
Expand All @@ -435,6 +444,16 @@ class cpp_function : public function {
if (options::show_function_signatures()) signatures += "\n";
}
}
if (rec->is_operator) {
signatures += "\n";
signatures += std::to_string(++index) + ". ";
signatures += rec->name;
signatures += "(";
if (rec->is_method) {
signatures += "self: " + self_type + ", ";
}
signatures += "*args, **kwargs) -> NotImplemented\n";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scrutinizing this again: NotImplemented is the value, and not the type, right. type(NotImplemented) prints <class 'NotImplementedType'> for me.

Also, is this the way it's done? Can you give me a reference of type annotations in pure Python code, perhaps?

Copy link
Contributor Author

@AWhetter AWhetter Nov 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I had incorrectly assumed that being a singleton, that NotImplemented was treated like None and that you're allowed to supply it. In fact, mypy doesn't use or need it at all (python/mypy#6710 (comment)) and so it isn't often used. Given that comment, Any seems like the correct type to return?
Alternatively, we could combine the two signatures into one and simply say that the type of the second argument is always object, because it will always be accepted and mypy will assume that NotImplemented will be returned regardless. It's less useful for a user that way, but it resolves the original issue without much extra complexity in the implementation?

}

/* Install docstring */
auto *func = (PyCFunctionObject *) m_ptr;
Expand Down
12 changes: 12 additions & 0 deletions tests/test_operator_overloading.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,15 @@ def test_overriding_eq_reset_hash():

assert hash(hashable(15)) == 15
assert hash(hashable(15)) == hash(hashable(15))


def test_docstring_includes_type_signature(msg):
"""#2269: Docstring includes a type signature of the additional overload"""
assert msg(m.Vector2.__eq__.__doc__) == """
__eq__(*args, **kwargs)
Overloaded function.

1. __eq__(self: m.operators.Vector2, arg0: m.operators.Vector2) -> bool

2. __eq__(self: m.operators.Vector2, *args, **kwargs) -> NotImplemented
"""