fix: say when a config file is skipped for lack of TOML support - #374
Open
Eljees wants to merge 1 commit into
Open
fix: say when a config file is skipped for lack of TOML support#374Eljees wants to merge 1 commit into
Eljees wants to merge 1 commit into
Conversation
…A#268) PyCQA#371 stopped docformatter crashing with a NameError when neither tomllib nor the tomli backport is importable, by returning early from _do_read_toml_configuration. That fixed PyCQA#368. What is left is PyCQA#268: the early return is completely silent, so a pyproject.toml -- including one passed explicitly with --config -- is ignored without a word, and the run looks like it simply chose to reformat differently. This is reachable on a supported configuration. pyproject.toml declares python = "^3.10", tomli is an optional extra for python < 3.11, and the CI matrix still runs 3.10 and pypy3.9. A plain `pip install docformatter` on 3.10 therefore reads no TOML configuration at all. Print one line to stderr naming the file that was not read and how to get TOML support, and leave the behaviour itself alone -- whether the settings should be honoured some other way is a separate question.
Contributor
Author
|
Flagging one thing so the red check is not read as mine:
I pulled both job logs and diffed them; the failing set is identical. The six test jobs (3.10 through 3.14 and pypy3.9) are green. The mypy one is worth a word since it sits in the file this PR edits: line 46 is the tomllib: ModuleType | None = Nonewith |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #268.
What is left of #268
#371 fixed the crash half of this: with neither
tomllibnor thetomlibackportimportable,
_do_read_toml_configurationused to blow up withNameError, and it nowreturns early instead. That closed #368.
The complaint in #268 is the other half, and it is still exact — the early return is
completely silent:
A
pyproject.tomlis ignored without a word, including one passed explicitly with--config. From the outside the run simply reformats differently than asked, which iswhat the reporter described: "it does not read the configuration from the config file,
simply omits it even if it's explicitly specified."
This is reachable on a supported configuration
pyproject.tomldeclarespython = "^3.10"tomliis an optional extra:tomli = {version = "^2.0.0", python = "<3.11", optional = true}ci.ymlstill runs3.10andpypy3.9, both below 3.11So a plain
pip install docformatteron Python 3.10 reads no TOML configuration at all,and says nothing about it.
Fix
One line to stderr naming the file that was not read, and how to get TOML support.
The behaviour itself is unchanged — the settings are still skipped. Whether they should
instead be honoured some other way is a separate question and not something I wanted to
decide inside a bug fix.
There is an alternative worth mentioning: making
tomlia required dependency forpython < 3.11is a one-line change inpyproject.tomland would remove the situationentirely. I did not do that because it changes the dependency graph for every user, which
felt like your call rather than mine. Happy to switch to that instead if you prefer it.
Tests
TestMissingTomlSupportintests/test_configuration_functions.py:test_says_so_when_toml_support_is_missingtest_stays_quiet_when_toml_support_is_presentThe second is the control — on the happy path stderr has to stay empty, and it does, both
before and after.
Verification
Run in a clean venv with the dependency set from
[testenv]intox.ini:master(d5c7b77), whole suiteblack --check,isort --checkon the changed sourcedocformatter --check --config ./pyproject.tomlon its own sourcetests/is excluded in.pre-commit-config.yaml, so the added test block was formattedon its own and the test diff is additions only.