astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
28.81k stars 935 forks source link

Make docstring convention a global setting #9043

Open Mr-Pepe opened 7 months ago

Mr-Pepe commented 7 months ago

Pydocstyle has a setting to specify the used docstring convention:

[tool.ruff.lint.pydocstyle]
convention = "google"

Pylint also has rules for docstrings and profits from setting the used convention. #8843 introduces one of those rules and adds a convention setting specific to Pylint.

I think that having two different settings for the convention is not very intuitive as a user and might hinder efforts to unify settings and rules later on if someone actually selects two different conventions for Pydocstyle and Pylint checks (no idea why someone would do that but there's always someone out there). Having a other tools, like a docstring formatter, in the future would further complicate things. The setting should therefore not only be global to linting, but global to all tools.

I propose to turn Pydocstyle's convention setting into a global setting to make it usable by the Pylint rules and other tools, like formatters. A strategy could look like the following:

MichaReiser commented 7 months ago

That makes sense to me. I wonder if it even should become a global (tool agnostic) setting, so that a docstring formatter could pick it up in the future.

Mr-Pepe commented 7 months ago

I had to read through some issues and PRs (#8449, #7549, #7651) to figure out what the current state and trend regarding the settings structure is. Making the setting global to ruff and not only linting seems reasonable if there are plans to make use of the setting in the formatter. I have updated my original post.

BrendanJM commented 5 months ago

Probably related to this (let me know if this is a separate issue), but pydocstyle allows for specifying convention using it's own config:

[tool.pydocstyle]
convention = "google"
match = '((?!_test).)*\.py'

There is some inconsistently currently because (at least from what I can tell), ruff would in this case appropriately use the pydocstyle config for match but disregards the convention config. Instead, we need to also set convention in a ruff-specific config

[tool.ruff.lint.pydocstyle]
convention = "google"

Without knowing too much on ruff's philosophy towards configs, it feels like the path of least complexity would be to rely on and respect the pydocstyle configs where applicable, since that allows single-sourcing configs between the two tools. Right now the mix of some configs being respected and others not feels like it could lead to confusion and potentially duplication.

zanieb commented 3 months ago

Generally our philosophy is that we will read Python standards i.e. [project] but not other tool's configuration formats.

BrendanJM commented 3 months ago

It makes sense, I just ran into this reading another issue regarding e.g. pip.conf and I see where you're coming from. But this philosophy is not without downsides, unfortunately, which are duplication and possible confusing configs.

It feels like it could be worth issuing a warning from ruff if other/conflicting configs are detected in pyproject.toml. Even something like "this warning is just a heads up that what happens might be different than what you expected", just to get people used to this paradigm, since you'd have to do some issue diving to understand this nuance.

BrendanJM commented 3 months ago

That said, there does seem to be some inconsistency - it's been a bit, but when I wrote that comment it appeared that ruff does use some of the configs from these other sections (match in this case), just not all of them.

zanieb commented 3 months ago

Yeah unfortunately there are trade-offs in both directions.

I'd be more willing to support a hint / warning if we detect the other configuration since then we're not as firmly on the hook for changes to the format.

it appeared that ruff does use some of the configs from these other sections (match in this case), just not all of them.

I'm honestly not familiar if it does. If you know where we read that could you link to the code? Our philosophy around these things is ever-evolving of course.

charliermarsh commented 3 months ago

... but when I wrote that comment it appeared that ruff does use some of the configs from these other sections (match in this case), just not all of them.

I'm very confident that we don't read match or anything from tool.pydocstyle -- I think it's ~impossible` since that's not encoded in our deserializer. Maybe there was something else going on?

BrendanJM commented 3 months ago

Apologies - you're correct. I had a separate config for ruff that was basically doing the same thing:

[tool.ruff.lint.per-file-ignores]
"test/*" = [
  "D"
]
zanieb commented 3 months ago

Adding a global setting is open for contribution.