Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Usage question: short docstring vs full-docstring #26

Closed
sobolevn opened this issue Jun 25, 2019 · 9 comments
Closed

Usage question: short docstring vs full-docstring #26

sobolevn opened this issue Jun 25, 2019 · 9 comments

Comments

@sobolevn
Copy link
Contributor

sobolevn commented Jun 25, 2019

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
Copy link
Owner

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

@terrencepreilly
Copy link
Owner

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
Copy link
Contributor Author

sobolevn commented Aug 6, 2019

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
Copy link
Owner

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 added a commit that referenced this issue Aug 10, 2019
Changed the strictness arguments to "short", "long", and "full", which
should be a little easier to specify and test on the command line.

Resolves: Issue #26
@terrencepreilly
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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.

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

No branches or pull requests

2 participants