terrencepreilly / darglint

A python documentation linter which checks that the docstring description matches the definition.
MIT License
482 stars 41 forks source link

Usage question: short docstring vs full-docstring #26

Closed sobolevn closed 5 years ago

sobolevn commented 5 years ago

Hi, thanks for this project! It is awesome!

My question is about two different modes of docstrings that I use in my apps. I can either write a short one like:

class SimpleViolation(BaseViolation):
    """Violation for cases where there's no associated nodes."""

    _node: None

    def __init__(self, node=None, text: Optional[str] = None) -> None:
        """Creates new instance of simple style violation."""
        super().__init__(node, text=text)

Or full one like here:

class BaseViolation(object):
    """
    Abstract base class for all style violations.

    It basically just defines how to create any error and how to format
    this error later on.

    Each subclass must define ``error_template`` and ``code`` fields.

    Attributes:
        error_template: message that will be shown to user after formatting.
        code: violation unique number. Used to identify the violation.

    """

    error_template: ClassVar[str]
    code: ClassVar[int]

    def __init__(self, node: ErrorNode, text: Optional[str] = None) -> None:
        """
        Creates new instance of abstract violation.

        Parameters:
            node: violation was raised by this node. If applied.
            text: extra text to format the final message. If applied.

        """
        self._node = node
        self._text = text

Full source here: https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/violations/base.py

The problem is that I get an error from the short version:

./wemake_python_styleguide/violations/base.py:168:1: I101 Missing parameter(s) in Docstring: - text
./wemake_python_styleguide/violations/base.py:168:1: I101 Missing parameter(s) in Docstring: - node

What do I suggest? Allow short docstring (= docstrings without ANY arguments/returns/raises/etc declarations) or require to write a full one (in case ANY formatters are found). It might be a configuration option turned off by default.

How does it sound to you?

terrencepreilly commented 5 years ago

That sounds reasonable, so long as it's not the default. It should be pretty easy to implement. I'll see what I can do (or if anyone else wants to tackle this, I can point them in the right spot to do it.)

sobolevn commented 5 years ago

@terrencepreilly awesome! Thanks you. As soon as this feature will be implemented and released I will add your package to our project: https://github.com/wemake-services/wemake-python-styleguide

sobolevn commented 5 years ago

@terrencepreilly I can help you with this task. Any guides about how to get started and where to look? Thanks!

terrencepreilly commented 5 years ago

Sorry for the late response: I was in a place with no internet access this whole weekend. So you could make this change by editing a couple of files. First, you'd want to edit config.py to add the option as a configuration option, and also driver.py to add a command line flag. Once you've done that, there's a file called integrity_checker.py, which does all of the checks for errors. You'd want to add a guard around here to check for that configuration option and whether the docstring has only one line. That should handle it, I think. Let me know if you run into problems.

sobolevn commented 5 years ago

Hi, @terrencepreilly! I am terribly sorry, but I don't have the time to work on this. The sad part is that this issue is a blocker for our adoption of this plugin in wemake-python-styleguide.

Roadmap

Here's the project roadmap:

  1. We are working on 0.12 release - it is the first public release of wemake-python-styleguide (that's why I am so busy at the moment)
  2. After it is released - it will be publicly announced for the first time on reddit, HN, etc
  3. Then we will have a feature freeze for half a year and we will focus on bugs and python3.8 support
  4. New features will only be added after python3.8 support will be released (including all plugins we depend on - it might take a while)

I would really love to see darglint on board - since it is awesome and completely matches our goal and style. And it would be awesome to fit into 0.12 with your help. 🙏

Feature description

We write a lot of docstrings. There are several types of them:

  1. Brief description without any details about arguments:
def some(arg: int) -> int:
     """This function does this and that."""
  1. Detailed description (sometimes with doctests):
def other(arg: int) -> int:
     """
     First line.

     Description.

     Examples:
     >>> other(1)
     2

     >>> other(2)
     3
     """
  1. Documented API (when it is important, like here: https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/checker.py#L95-L120 where we describe flake8 plugin API):
def __init__(
        self,
        tree: ast.AST,
        file_tokens: Sequence[tokenize.TokenInfo],
        filename: str = constants.STDIN,
    ) -> None:
        """
        Creates new checker instance.

        These parameter names should not be changed.
        ``flake8`` has special API that passes concrete parameters to
        the plugins that ask for them.
        ``flake8`` also decides how to execute this plugin
        based on its parameters. This one is executed once per module.

        Parameters:
            tree: ``ast`` parsed by ``flake8``.
                Differs from ``ast.parse`` since it is mutated by multiple
                ``flake8`` plugins. Why mutated? Since it is really expensive
                to copy all ``ast`` information in terms of memory.
            file_tokens: ``tokenize.tokenize`` parsed file tokens.
            filename: module file name, might be empty if piping is used.
        """

So, I guess there might be a setting to allow certain level of details before checking actual docs.

  1. Default one: all docstrings must specify parameters docs
  2. One-liners are allowed, anything else requires API description
  3. One-liners and long docstrings without Parameters, Raises, Yields, Returns sections are allowed. If these sections are present - then we need to check everything

These levels are sorted from the strictest to the mildest.

Thanks for your wonderful work! 👍 Would be happy to include this plugin to our project.

terrencepreilly commented 5 years ago

No problem. I just finished rewriting the parser, and was about to merge it in, so I was hesitant to make too many changes. I've got to do some regression testing and cleanup anyway, so I'll see if I can squeeze it in this week some time.

terrencepreilly commented 5 years ago

I've implemented this under the flag/configuration option, "strictness". You should be able to put

[darglint]
strictness=short

In your configuration to allow for one-line docstrings. (If the docstring has anything more than one line, it will be checked.) You can also specify it by the command-line argument, --strictness or -z:

darglint -z full example.py

There's a more complete description, as well as examples of how this changes the checking of docstrings, in the README, here.

Let me know if it doesn't work, or if it's unexpected in some way. I'm going to freeze any other feature work until I've merged in the new parser.

Resolved in v0.6.0

sobolevn commented 5 years ago

AWESOME! I would like to buy you a beer/pizza/coffee if that's possible. (paypal, opencollective, patreon, bitcoins, etc)

You have saved so much time for me! Thanks a lot! 🤝

terrencepreilly commented 5 years ago

That's kind of you. I haven't set anything up: this is just a fun project. Maybe I will at some point in the future.