Skip to content
This repository was archived by the owner on Nov 3, 2023. It is now read-only.

Wrong “D103: Missing docstring in public function” for overload functions #419

Closed
raabf opened this issue Oct 17, 2019 · 9 comments · Fixed by #511
Closed

Wrong “D103: Missing docstring in public function” for overload functions #419

raabf opened this issue Oct 17, 2019 · 9 comments · Fixed by #511

Comments

@raabf
Copy link

raabf commented Oct 17, 2019

For example:

from typing import overload

@overload
def func(a: int) -> str:
     ...
@overload
def func(a: str) -> str:
     ...
def func(a):
     """Foo bar documentation."""
     return str(a)

will result in

/home/raabf/Dokumente/overload_test.py:4 in public function `func`:
    D103: Missing docstring in public function
/home/raabf/Dokumente/overload_test.py:7 in public function `func`:
    D103: Missing docstring in public function

Which is wrong since only the last def should have a docstring.

Two things should be done:

First, prevent the error D103 for overload functions. We could make use of the --ignore-decorators='overload', but since it is universally valid, it should be a fixed part of it, and the user should not set it by himself.

Second, since @overload functions should never have docstrings, it would be useful if there is an additional error code in pydocstyle, if someone is setting a docstring for an @overload function.

@Nurdok
Copy link
Member

Nurdok commented Dec 1, 2019

Both ideas sound good to me. PRs welcome.

@Nurdok Nurdok added Minor (New Feature) Waiting for Assignee This issue has been triaged as a good idea, waiting for a volunteer to implement labels Dec 1, 2019
@rmorshea
Copy link
Contributor

rmorshea commented Feb 28, 2020

This is a big issue for anyone using type annotations since turning off the error prevents you from ensuring you have documentation coverage. @Nurdok, given this drawback, resolving the issue seems to be of slightly higher priority.

@rmorshea
Copy link
Contributor

I've just noticed that you only have two priority tags so maybe this doesn't rise to the level of "major", but it's worth considering that turning off the error negates the huge benefit of guaranteeing doc coverage.

@thomasgilgenast
Copy link

thomasgilgenast commented Jul 11, 2020

I am also encountering this.

Instead of turning off D102/D103 completely, I think it's possible to add # noqa: D102 or # noqa: D103 to the @overload definitions (but not the real implementation) as a temporary workaround.

@theyuvalraz
Copy link
Contributor

I maybe biting more than I can chew. But I think I will take a stab at this.

If I understand correctly what is needed here is:

  1. D103 shouldn't be triggered if the function is decorated with @overload.
  2. If a function is decorated with @overload and has a docstring then it should throw a new error code.

@theyuvalraz
Copy link
Contributor

@samj1912
About the second objective: Adding a new error code for "Function decorated with @overload shouldn't contain a docstring"

This does not fit any of the grouping available, Should I create D5XX "shouldn't contain docstring" group?

@sambhav
Copy link
Member

sambhav commented Sep 6, 2020

I think D4xx may still be a good candidate for this error

@sambhav sambhav removed the Waiting for Assignee This issue has been triaged as a good idea, waiting for a volunteer to implement label Sep 6, 2020
@jdtzmn
Copy link

jdtzmn commented Nov 23, 2020

Is this also fixed for D102? I'm still getting D102 errors on overloaded functions.

@ShadowLNC
Copy link

I'm not seeing D418 errors with the following file:

"""Reproduce D418."""

from typing import overload, Any

@overload
def func(val: str) -> str:
    ...

@overload
def func(val: int) -> int:
    """Return original value."""
    ...

def func(val: Any) -> Any:
    """Return original value."""
    return val

Should I be expecting to see them? pydocstyle --version gives 6.0.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants