darglint performance on Google style docstrings may be too slow to be a pre-commit hook

[paw-lu]

darglint's performance on Google-style docstrings is pretty slow on my machine¹. It gets even slower when you try to integrate rst into the docstring.² Slow enough that in some cases it's too much friction as a pre-commit hook (at least on my machine). Posting this both to document my issue and to see if others have ran into this.

Example

"""bar.py"""
def foo():
    """Summary

    Args:
        file (Any):
            A path to a Jupyter Notebook file.
        theme (str): The theme to use for syntax highlighting. May
            be ``'ansi_light'``, ``'ansi_dark'``, or any
            `Pygments theme`_. By default ``'ansi_dark'``.
        plain (bool): Only show plain style. No decorations such as
            boxes or execution counts. If set to pydata`None`
            will autodetect. By default pydata`None`.
        unicode (Optional[bool]): Whether to use unicode characters
            to render the notebook. If set to pydata`None` will
            autodetect. By default pydata`None`.
        hide_output (bool): Do not render the notebook outputs. By
            default pydata`False`.
        nerd_font (bool): Use nerd fonts when appropriate. By
            default pydata`False`.
        files (bool): Create files when needed to render HTML
            content. By default pydata`True`.
        negative_space (bool): Whether render character images in
            negative space. By default pydata`True`.
        hyperlinks (bool): Whether to use hyperlinks. If
            pydata`False` will explicitly print out path. If set
            to pydata`None` will autodetect. By default
            pydata`None`.
        hide_hyperlink_hints (bool): Hide text hints of when content
            is clickable. By default pydata`False`.
        images (Optional[str]): Whether to render images. If set to
            pydata`None` will autodetect. By default
            pydata`None`.
    """
    ...

% darglint bar.py

This example takes 52 seconds on my machine. The example from the issue takes 2:02.² This can evolve into a bigger issue—as heavily docstringed commits were timing out on my machine when darglint was ran as a pre-commit hook.

Possible solutions

1. Switch the docstring styles to Sphinx Style
2. Remove darglint as a pre-commit hook (but keep it as a CI linting session)
3. Remove darglint as a default linter from the project

Footnotes

1. https://github.com/terrencepreilly/darglint/issues/186 ↩

2. https://github.com/terrencepreilly/darglint/issues/198 ↩ ↩²

[cjolowicz]

Thanks! Ideally we'd get this fixed upstream (my first guess would be catastrophic regex backtracking). As a temporary workaround, option 2 could be done by marking the hook manual.

As a sidenote: The MyST project is working on their own Markdown docstring support. But that may take a while, and whatever syntax they end up with may still be susceptible to this problem.

[paw-lu]

As a temporary workaround, option 2 could be done by marking the hook manual.

Yeah, I like this approach the most. But the trouble I'm having with it is that even when DAR is explicitly ignored, it still actually runs in the background, slowing down the linting process. As far as I know, there is no way to actual disable a flake8 plugin—just ignore its warnings¹

So it seems that as long as darlint and flake8 are installed in the same environment (the nox pre-commit env) flake8's performance will be impacted. Maybe darlint can have its own nox session (like we do with mypy, typeguard, xdoctest)?

What this approach does not solve is that the editor's linting process will still be impacted by the performance—since darglint will be in the general dev environment anyways.

Footnotes

1. https://github.com/PyCQA/flake8/issues/751 ↩

[cjolowicz]

So I guess we'd have to run darglint as a stand-alone pre-commit hook, separate from flake8 and outside of the Nox and Poetry environments, i.e. as a regular (non-system) hook managed by pre-commit.

[cjolowicz]

The thing about having a separate Nox session is that we'd also get an additional CI job then. And it would be surprising to have it in Nox but not Poetry.

TBH I don't like any of the workarounds much...

[paw-lu]

TBH I don't like any of the workarounds much...

Yeah I'm 100% with you here. Still trying to think of an elegant solution.

The best I can think of is what you already mentioned—a stand-alone hook.

repos:
-   repo: https://github.com/terrencepreilly/darglint
    rev: master
    hooks:
    - id: darglint
      stages: [manual]

[paw-lu]

Maybe the best option (aside from fixing the performance issue upstream) is actually to step back and open a PR that makes darglint's flake8 integration off by default.

[paw-lu]

Maybe the best option (aside from fixing the performance issue upstream) is actually to step back and open a PR that makes darglint's flake8 integration off by default.

Notes for myself on how to do this.

• Example off-by-default plugin
• flake8 entrypoint for darglint

[paw-lu]

Though unfortunatley this would now require projects to explictitly enable the integration by default, and would quietly disable darglint on current linting workflows. So I completely understand if the maintainer would not want to adopt this.

[cjolowicz]

Could darglint to be changed to let users opt out off flake8 integration, rather than changing its default?

[cjolowicz]

Ideally we'd get this fixed upstream (my first guess would be catastrophic regex backtracking)

Guessed wrong.

snip...

moved the rest of this comment here

snip...

All in all, not a trivial fix.

[paw-lu]

Could darglint to be changed to let users opt out off flake8 integration, rather than changing its default?

Yeah maybe this is the least disruptive change. There's a good spot for this here. We could just add something like a disable_flake8 option for .darglint and check for its existance there.

[paw-lu]

Yet another work around option is that darlgint adds a --darglint-ignore-regex option to flake8.

So the pre-commit hook can have something like

      - id: flake8
        name: flake8
        entry: flake8
        language: system
        types: [python]
        require_serial: true
        args: [--darglint-ignore-regex, .*]

[cjolowicz]

Yes, it seems this option was added for precisely our use case: terrencepreilly/darglint#176

So I guess we could do the following:

1. Exclude darglint from the flake8 hook using --darglint-ignore-regex (your code above)
2. Add a dedicated darglint hook that runs only in the manual stage.
3. Change the Nox session so the manual stage is included when CI is present in the environment.

[paw-lu]

Implimentation attempted in cjolowicz/cookiecutter-hypermodern-python-instance#822.