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

Autofix remove types from docstring #9070

Open
KennethEnevoldsen opened this issue Dec 9, 2023 · 6 comments
Open

Autofix remove types from docstring #9070

KennethEnevoldsen opened this issue Dec 9, 2023 · 6 comments
Labels
rule Implementing or modifying a lint rule

Comments

@KennethEnevoldsen
Copy link

KennethEnevoldsen commented Dec 9, 2023

Currently types can be added in two places in a function as a type hint and in the documentation.

def myfunc(value: int):
   """
   Args:
      value (int): any value you want
   """
   pass

This can lead to potential conflict between the type hint and the docstring leading to multiple sources of truth. #5541 seeks to solve this by fixing the docstring, however this could still lead to the reverse issue; that the docstring is updated but not the type hint. It also avoid duplicating information. The reason to prioritise the type hint is meaningful as it can be checked by type hinters etc.

There might still be reasons to have types in the docstring such as documentation, but there already exist extension which remedy this need (e.g. https://pypi.org/project/sphinx-autodoc-typehints/). I probably wouldn't set it as a default though.

Let me know if there is something I have missed. I very much enjoy the package.

Edit: In a similar vein, it might also be worth discouraging "Defaults to {value}" in the docstring as it similar can be outdated and is already specified function signature.

@sbrugman
Copy link
Contributor

sbrugman commented Dec 9, 2023

For the same reasoning we also dropped type information from our docstrings. The redundancy is error prone and adds little information.

Another similar rule would be type docstrings for classes, where the type hint can be provided by instance variable annotations (example)

@Mr-Pepe
Copy link
Contributor

Mr-Pepe commented Dec 10, 2023

Great idea! At work, we switched to sphinx-autodoc-typehints but now we have lots of code that is inconsistent, with old docstrings containing the type and new code omitting it.

@T-256
Copy link
Contributor

T-256 commented Dec 10, 2023

Also PEP 727 maybe could change the way of how to document parameters. then, after it standardized, we need to remove junks from docstrings.

@Mr-Pepe
Copy link
Contributor

Mr-Pepe commented Dec 10, 2023

In order to implement an autofix, a new rule is needed first, right? Something like Parameter type already documented via type hint? This would probably fit into the D4XX category.

Also PEP 727 maybe could change the way of how to document parameters. then, after it standardized, we need to remove junks from docstrings.

I think that is orthogonal to this issue and should be treated separately, once PEP 727 has been accepted. The rule could be something like Parameter already documented in function signature.

@zanieb zanieb added the rule Implementing or modifying a lint rule label Dec 10, 2023
@zanieb
Copy link
Member

zanieb commented Dec 10, 2023

This seems reasonable to me! Although we do not yet have a dedicated docstring parser and may want to wait until we write one.

@sbrugman
Copy link
Contributor

See also #458

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

5 participants