Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Enable full toml configuration and pyproject.toml #534

Merged
merged 17 commits into from
May 9, 2021
Merged

Enable full toml configuration and pyproject.toml #534

merged 17 commits into from
May 9, 2021

Conversation

RuRo
Copy link
Contributor

@RuRo RuRo commented May 1, 2021

This PR closes #447.

Changes

  • Fix a small unrelated bug, where inline comments in .ini files behaved differently when autodetected by _get_config_file_in_folder and when manually specified by --config.

  • Add toml as a package dependency (setup.py) and as a pinned runtime dependency (requirements/runtime.txt).

  • Implement TomlParser - a thin wrapper around the toml library with minimal API parity with RawConfigParser.

  • Autodetect pyproject.toml configuration

  • Autodetect tool.pydocstyle section

  • Implement error code list support for toml. This is so that you can use

    select = [
        "D100",
        # This line is ignored
        "D103",
    ]

    and not only

    select = "D100,D103"
  • Almost all tests in test_integration.py which used the env fixture now are run for both toml and ini versions of the configuration (47 tests).

  • 2 tests are only run for ini configuration: test_ignores_whitespace_in_fixed_option_set and test_multiple_lined_config_file. In toml, strings must be quoted, so comments and newlines inside the strings don't make much sense, which is what these tests are checking. Instead, I've implemented 2 "equivalent" toml tests that also check the list support: test_accepts_ignore_error_code_list and test_accepts_select_error_code_list.

Please make sure to check for the following items:

  • Add unit tests and integration tests where applicable.
  • Add a line to the release notes (docs/release_notes.rst) under "Current Development Version".
    Make sure to include the PR number after you open and get one.

There's currently no "Current Development Version" section in docs/release_notes.rst. Where should I add the changes?

src/pydocstyle/config.py Outdated Show resolved Hide resolved
src/tests/test_integration.py Outdated Show resolved Hide resolved
src/pydocstyle/config.py Outdated Show resolved Hide resolved
src/pydocstyle/config.py Outdated Show resolved Hide resolved
@RuRo
Copy link
Contributor Author

RuRo commented May 7, 2021

@Nurdok, sorry to bother you, but can you maybe take a look at this PR?

@Nurdok
Copy link
Member

Nurdok commented May 8, 2021

@samj1912 is the current active maintainer - I'll let him address this PR.

@sambhav
Copy link
Member

sambhav commented May 8, 2021

Will take a look this weekend :)

@sambhav
Copy link
Member

sambhav commented May 9, 2021

This generally looks good to me, just a couple of comments -

  1. Can we make toml an optional dependency so that if it's not present, people can still use pydocstyle. This is so that you can install pydocstyle with toml support using something like pip install pydocstyle[toml] or if you don't want to have support for it, you can just use pip install pydocstyle. Incase toml is not present as a dependency and pyproject.toml is, pydocstyle could show a warning that it maybe missing user configuration due to the lack of toml dependency and that users should install toml to get support for it.
  2. Can we have the section names conditional to the file being parsed? I would prefer if we only parsed the tool.pydocstyle section in pyproject.toml and not the other top level sections.

@RuRo
Copy link
Contributor Author

RuRo commented May 9, 2021

  1. Theoretically we can, however it's not clear to me, what should happen if the pyproject.toml file is present, but toml is not installed.

    • Raising an error might break existing projects that currently have a pyproject.toml file and either don't have a .pydocstyle or have it in a subdirectory instead of in the root of the project.
    • Ignoring the pyproject.toml file if toml is not installed can lead to unexpected results, since toml can also be installed as a dependency of some other package or can be installed on the developers machine, but not in CI or vice-versa.
  2. Done. Just to be clear,

    I would prefer if we only parsed the tool.pydocstyle section in pyproject.toml and not the other top level sections.

    toml will still technically parse the whole pyproject.toml file no matter what. I just changed it so that the pydocstyle configuration keys are only looked up in tool.* instead of also checking for pydocstyle/pep257 in the root.

@sambhav
Copy link
Member

sambhav commented May 9, 2021

  1. Theoretically we can, however it's not clear to me, what should happen if the pyproject.toml file is present, but toml is not installed.

    • Raising an error might break existing projects that currently have a pyproject.toml file and either don't have a .pydocstyle or have it in a subdirectory instead of in the root of the project.
    • Ignoring the pyproject.toml file if toml is not installed can lead to unexpected results, since toml can also be installed as a dependency of some other package or can be installed on the developers machine, but not in CI or vice-versa.

I meant something similar to this - https://github.com/cruft/cruft/pull/107/files

  1. Done. Just to be clear,

    I would prefer if we only parsed the tool.pydocstyle section in pyproject.toml and not the other top level sections.

    toml will still technically parse the whole pyproject.toml file no matter what. I just changed it so that the pydocstyle configuration keys are only looked up in tool.* instead of also checking for pydocstyle/pep257 in the root.

Yup, this is what I meant :).

@RuRo
Copy link
Contributor Author

RuRo commented May 9, 2021

I meant something similar to this - https://github.com/cruft/cruft/pull/107/files

Sure, I guess a warning is an acceptable compromise.

@sambhav
Copy link
Member

sambhav commented May 9, 2021

Code wise it all looks good :) Could you also update the changelog with an entry for this PR and if possible update the docs! Thanks

@RuRo
Copy link
Contributor Author

RuRo commented May 9, 2021

Could you also update the changelog with an entry for this PR

As I mentioned in the original post

There's currently no "Current Development Version" section in docs/release_notes.rst. Where should I add the changes?

@sambhav sambhav merged commit 8d8b319 into PyCQA:master May 9, 2021
@sambhav
Copy link
Member

sambhav commented May 9, 2021

Thank you for this PR :)

@RuRo
Copy link
Contributor Author

RuRo commented May 9, 2021

👍

Btw, I was wondering, does pydocstyle have a release schedule? Do you have any estimations of when might the next version be released? I'm itching to bump the version in my personal projects :^)

@sambhav
Copy link
Member

sambhav commented May 9, 2021

It does not have a fixed release schedule. I can cut a release sometime next week though :)

@RuRo
Copy link
Contributor Author

RuRo commented May 10, 2021

That would be much appreciated.

@Nytelife26
Copy link

Nice work :) turns out it wasn't at all as complex as implied, eh. That, or you're a software engineer from beyond modern earth.

All jokes aside, thank you for this - I'll be using this feature in proselint shortly too

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

Successfully merging this pull request may close these issues.

pyproject.toml configuration support
4 participants