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

Use tomllib/tomli for reading .toml configs #608

Merged
merged 3 commits into from Jan 3, 2023
Merged

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Oct 12, 2022

Use the built-in tomllib module in Python 3.11 and the modern tomli package in older Python versions to read .toml configs instead of the unmaintained and broken toml package.

Fixes #599
Fixes #600


Thanks for submitting a PR!

Please make sure to check for the following items:

  • Add unit tests and integration tests where applicable. (n/a)
    If you've added an error code or changed an error code behavior,
    you should probably add or change a test case file under tests/test_cases/ and add
    it to the list under tests/test_definitions.py.
    If you've added or changed a command line option,
    you should probably add or change a test in tests/test_integration.py.
  • 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.

Please don't get discouraged as it may take a while to get a review.

Use the built-in `tomllib` module in Python 3.11 and the modern `tomli`
package in older Python versions to read .toml configs instead of
the unmaintained and broken `toml` package.

Fixes PyCQA#599
Fixes PyCQA#600
@mgorny
Copy link
Contributor Author

mgorny commented Jan 3, 2023

Rebased on top of poetry.

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this depend on #612 to be really tested, could you rebase on it ?

@samj1912 samj1912 dismissed Pierre-Sassoulas’s stale review January 3, 2023 09:04

The Python 3.11 change was merged in and this PR is being properly tested now.

@samj1912
Copy link
Member

samj1912 commented Jan 3, 2023

Thanks for this PR!

@samj1912 samj1912 merged commit 3bc3b87 into PyCQA:master Jan 3, 2023
@mgorny
Copy link
Contributor Author

mgorny commented Jan 3, 2023

Thanks!

@mgorny mgorny deleted the tomli branch January 3, 2023 10:52
@christianbundy
Copy link

This was a breaking change for our team, btw -- we're on Python 3.10 and have had toml installed, but updating to the latest version meant that our configuration file was ignored and so we got a bunch of unexpected errors:

WARNING: The pyproject.toml configuration file was ignored, because the `tomli` package is not installed.

Now that I understand what changed, it's a simple fix, but wanted to let y'all know that this wasn't 100% backward-compatible. Either way, thanks for the improvement -- I'm increasingly excited to join the Python 3.11 gang.

@samj1912
Copy link
Member

@christianbundy how were you installing pydocstyle with toml support? Were you using pydocstyle[toml] ? If so, it should not have been a breaking change.

@christianbundy
Copy link

We're using pre-commit, here's the config we were using:

  - repo: https://github.com/pycqa/pydocstyle
    rev: 6.2.3 # previously using 6.1.1, upgrading caused errors
    hooks:
      - id: pydocstyle
        args:
          - --config=pyproject.toml
        additional_dependencies:
          - toml~=0.10.2

Should we have been doing this differently? I think maybe we could add pydocstyle[toml] as an additional dependency, but I think that would install pydocstyle twice? Not sure how that would work.

@samj1912
Copy link
Member

@christianbundy you want the following -

pre-commit/pre-commit#1526 (comment)

@christianbundy
Copy link

Oh, nice! Thank you.

@mgorny
Copy link
Contributor Author

mgorny commented Jan 13, 2023

@christianbundy you want the following -

pre-commit/pre-commit#1526 (comment)

Could someone ask upstream to include that in the official documentation? It's not the most predictable way of sorting this particular problem. Unfortunately, I can't do that since upstream likes banning people for no apparent reason.

@scottstanie
Copy link

scottstanie commented Feb 2, 2023

Just to spell it out for any others, it took me a few tries to connect the comment above to what we need here. My .pre-commit.yaml now has

  - repo: https://github.com/PyCQA/pydocstyle
    rev: "6.3.0"
    hooks:
      - id: pydocstyle
        additional_dependencies: ['.[toml]']

and it made the tomli error go away

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.

Pydocstyle crashes on literal strings in pyproject.toml Also allow other toml file readers
5 participants