Add support for overloaded functions

[ciscorn]

This PR adds basic support for the typing.overload decorator.

It works by storing all the overloaded signatures into the first definition.

Input:

class Double:
    """This is a class"""
    @overload
    def double(self, a: int) -> int: ...
    @overload
    def double(self, a: str) -> str: ...
    def double(self, a: Union[int, str]) -> Union[str, int]:
        """This is an overloaded method"""
        return a * 2

@overload
def double(a: int) -> int: ...
@overload
def double(a: str) -> str: ...
def double(a: Union[int, str]) -> Union[int, str]:
    """This is an overloaded function"""
    return a * 2

Output:

Please check the tests/python/py3example for details.

(This PR also fixes the issue that the autoapi silently drops duplicated methods in the same class.)

Close #217

[AWhetter]

This is excellent! Thanks for the changes. I've made some suggestions. If you don't want to tackle the refactoring then feel free to leave that and we can do it after this is merged.

[AWhetter]

What is the advantage of using the unicode values here?

[ciscorn]

Both → and \u2192 are OK. I just copied &#x2192 from Sphinx's this line.

By the way, the original autosummary doesn’t print return type annotations (maybe for saving spaces?).

[AWhetter]

I wonder why that is. But you're right to match what Sphinx is doing so let's stick with what's here!

[AWhetter]

I'm not aware of any real convention around how the docstring of an overloaded function should be displayed. The most common way I've seen is the way that pybind does it.

"""my_method(*args, **kwargs) -> Any

This is the docstring for the primary definition.

1. my_method(arg0: str) -> None

This is the docstring for the first overload.

2. my_method(arg0: bool) -> None

This is the docstring for the second overload.

I don't think we'd need that first signature because we'll have already defined it in the :py:method or :py:function directive, but do you think that it would make the docstring more clear if we formatted it in this way?

This comment applies to the additions in parse_module as well.

[ciscorn]

I totally agree that we shouldn't write docstrings in overload definitions in standard .py (not .pyi) code. There is no way to access overloads’ docstring at runtime.

The only reason my code concatenates docstrings is to allow us to write documentation in .pyi stubs that cannot have actual implementations.

FYI: Autodoc doesn't support .pyi yet and it completely drops docstrings of overload definitions.

[AWhetter]

Let's stick with ignoring the docstrings of overloads and use only the docstring of the actual implementation. It seems like overloads aren't supposed to have docstrings, plus Sphinx doesn't render them that way. Authors then have more control over how the docstring is rendered because they'll be formatting it however they like in the single primary docstring.

[ciscorn]

Autoapi's pyi support is very useful and important for C-extension libraries. (Autodoc is also trying to support it (sphinx-doc/sphinx#4824))

If we completely ignore docstrings of overloads, we will be unable to write docs on Python stubs because stubs are not allowed to have actual implementations. I'll show an example:

test_invalid.pyi

from typing import overload, Union

def hello(a: int) -> int:
    """This is okay"""

@overload
def double(a: int) -> int: ...
@overload
def double(a: str) -> int:
def double(a: Union[str, int]) -> int:
    """DOCSTRING"""

run mypy test.pyi:

test_invalid.pyi:10: error: An implementation for an overloaded function is not allowed in a stub file
Found 1 error in 1 file (checked 1 source file)

So we have to write the docstring in an overload.

test_valid.pyi:

from typing import overload, Union

def hello(a: int) -> int:
    """This is okay"""

@overload
def double(a: int) -> int: ...
@overload
def double(a: str) -> int:
    """DOCSTRING"""

run mypy --strict test_valid.pyi

Success: no issues found in 1 source file

Do you have any ideas about this?

I think employing only the last docstring is better than concatenating the all.

[AWhetter]

When using stubgen to generate a stub file for a C extension written with pybind, it sorts the actual implementation to be the last.
So I agree. Let's use only the last docstring when there are no implementations of the function/method.

[AWhetter]

Would it make more sense to store the overloads in an attribute instead of storing the real function signature with it as well? I feel like the overloads more of a special case of the function.

[ciscorn]

I can’t make a design decision, so I just emulated the Autodoc’s rendering [the figure below].

Python's @overload is nothing more than additional type hints for a single implementation. I think It's different from the real overloading in languages like C/C++.

[AWhetter]

I think the result that you have looks good. What I meant was that we should store overloads as an attribute called "overloads" on the objects, whereas at the moment you have an attribute called signatures that stores all signatures.
So the template would then look like

.. function:: {{ obj.short_name }}({{ obj.args }}){% if obj.return_annotation is not none %} -> {{ obj.return_annotation }}{% endif %}

{% for (args, return_annotation) in obj.overloads %}
              {{ obj.short_name }}({{ args }}){% if return_annotation is not none %} -> {{ return_annotation }}{% endif %}

{% endfor %} 

[ciscorn]

@AWhetter Thanks for the review!
I've extracted the large and common loop into a function. Also, I provided additional explanations in the comments.

[ciscorn]

I made the following changes:

• Splited the first signature (obj.args, obj.return_annotation) from the rest (obj.overloads). The code and templates has became simpler.
• It now picks a docstring only from either the actual implementation or the last overload definition.

[AWhetter]

This looks good to me. Thanks for the amazing work!