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

Make docstring convention a global setting #9043

Open
Mr-Pepe opened this issue Dec 7, 2023 · 10 comments
Open

Make docstring convention a global setting #9043

Mr-Pepe opened this issue Dec 7, 2023 · 10 comments
Labels
configuration Related to settings and configuration help wanted Contributions especially welcome

Comments

@Mr-Pepe
Copy link
Contributor

Mr-Pepe commented Dec 7, 2023

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:

  • Add a docstring-convention setting under [tool.ruff]
  • Deprecate the Pydocstyle setting (and Pylint's if [pylint] Add missing return doc rule (W9011) #8843 gets merged in its current form) in favor of the new setting
  • Only mention the global setting in the docs
  • Remove the original setting at some point in the future (is Ruff's deprecation process based on time or releases?)
@MichaReiser
Copy link
Member

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

Mr-Pepe commented Dec 8, 2023

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.

@dhruvmanila dhruvmanila added the configuration Related to settings and configuration label Dec 8, 2023
@BrendanJM
Copy link

BrendanJM commented Jan 14, 2024

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

zanieb commented Mar 12, 2024

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

@BrendanJM
Copy link

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

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

zanieb commented Mar 12, 2024

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

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

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 zanieb added the help wanted Contributions especially welcome label Mar 13, 2024
@zanieb
Copy link
Member

zanieb commented Mar 13, 2024

Adding a global setting is open for contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Related to settings and configuration help wanted Contributions especially welcome
Projects
None yet
Development

No branches or pull requests

6 participants