Search for pyproject.toml config file in parent dirs

[Felixoid]

Type of Changes

	Type
✓	✨ New feature

Description

Search for pyproject.toml in parent directories.

Closes #3289

[coveralls]

Pull Request Test Coverage Report for Build 3014845916

• 17 of 17 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.004%) to 95.338%

---

Totals
Change from base Build 3008071684:	0.004%
Covered Lines:	17033
Relevant Lines:	17866

---

💛 - Coveralls

[Felixoid]

How would I add exceptions for false-positive spell checker?

[DanielNoord]

Thanks for the interest in this issue @Felixoid. I didn't notice we left that issue open, but due to discussion in #3769 I am not sure if we can merge this PR.
Even if we do want to I think we should consider whether this is a breaking change and if it can be included in 2.x.

[Pierre-Sassoulas]

I think this should use the specification from the discussion in #3289

[Pierre-Sassoulas]

How would I add exceptions for false-positive spell checker?

There's a dictionary called .pyenchant_pylint_custom_dict.txt at the root of the project.

[Felixoid]

Hi @DanielNoord, there's nothing relatively new in the code that wasn't there already. The snippet curdir = Path(os.getcwd()).resolve() is a bit below, so it should work fine.

Dear @Pierre-Sassoulas, you mean that .git/.hg as another marker for the pyproject.toml directory? Or the max depth? Or rather that it should search for any possible configuration up to a repository root directory?

[Pierre-Sassoulas]

We handle multiple configuration file (pylintrc/.pylintrc / pyproject.toml, setup.cfg too) I think we should upgrade the current function to search for configuration files to make it search up the tree until it hits a configuration file or a .git / .hg directory. Unless I'm mistaken it's this function that need to be modified:

https://github.com/PyCQA/pylint/blob/2e2f8f8c12a2e54c18c0e4dd49e4850f5c913b1f/pylint/config/find_default_config_files.py#L42

[Felixoid]

Thank you for your review, I've added processing of all supported configs, and updated the docs

[Felixoid]

It definitely went out of control. I'll take care of it.

[DanielNoord]

The failing tests show that this is not a trivial change. Again, I'm against changing this in a 2.x update. Consider:

A directory without a git repo and no pyproject.toml. For example, a collection of (internal) scripts.
An user can now run pylint on this directory and get the default values and pass the checks.
With this change we would now start going up until the user's root, thereby potentially changing the config file that is being used for their checks.

I'm usually fine with having users change some configuration stuff between minor patches, but forcing them to restructure their directories or move files around doesn't seem like a good thing to do in a minor version update.

[Felixoid]

Ok, I misinterpret your first message. I thought that you've meant python2.

You are right on one hand. On another, now nobody can set a project global pylintrc, of there is a mixed repo. That's why I started working on the issue, basically

[DanielNoord]

Ok, I misinterpret your first message. I thought that you've meant python2.

You are right on one hand. On another, now nobody can set a project global pylintrc, of there is a mixed repo. That's why I started working on the issue, basically

See L62-80 of the original code. We currently support an environment variable and a global config in the users home directory. This probably doesn't cover all cases (and it might still make sense to merge this PR at some point), but I'm not sure we can change this now.

I haven't tested this myself but can't you also use pylint --rcfile=../../../pylintrc?

[Felixoid]

It's the morning, so I'll address all your questions.

With this change we would now start going up until the user's root, thereby potentially changing the config file that is being used for their checks.

It's precisely the issue I am trying to solve. And to show that it works, we already have the best proof. Take a look on the report. Each tested project has less number of reports than it has. It works as expected.

See L62-80 of the original code

Unfortunately, it's not even a workaround to have a repo-wide config. One can't set it for integrations, like me using ale for vim.

What is in my mind for now:

pylint has a severe issue with having a really basic feature of setting a repo-wide config. I 100% see your point with breaking change of looking for all types of config through the directories up to the root. What can be done, for now I'd roll back to the approximate state from the commit and add .git/.hg there only for pyproject.toml configuration. It's not such a significant change, and one is looking for a way to set a repo-wide config will have it.

The major change to search all configs in the directories tree may be then addressed in a major release.

Does it sound reasonable?

[DanielNoord]

It's precisely the issue I am trying to solve. And to show that it works, we already have the best proof. Take a look on the report. Each tested project has less number of reports than it has. It works as expected.

This is an unrelated report and caused by a bug in our primer. Actually I would expect this PR to have no effect in the primer as all primer projects are linted with a specified pylintrc. For now we should ignore all primer comments in this PR until #7160 has been merged.

What can be done, for now I'd roll back to the approximate state from the commit and add .git/.hg there only for pyproject.toml configuration. It's not such a significant change, and one is looking for a way to set a repo-wide config will have it.

The major change to search all configs in the directories tree may be then addressed in a major release.

Does it sound reasonable?

This is still a breaking change for those who want to lint subdirectories with default values. I can't stress enough the range of different pylint invocations. Take for example #6799, which shows that some users actually run pylint twice over the same repository with the same configuration file but with different command line arguments. Initially that seemed totally bizarre to me, but I guess people find a use case for it. Same goes for linting subdirectories with default values, we just don't know whether users rely on this.

I can see the need for repository wide configuration files, but I don't think this can be default value in a minor release. Perhaps we can work with a flag? --look-for-parent-configuration or something?

[Felixoid]

Sorry, I can't call it breaking changes. For me, it is still an essential function that wasn't implemented for historical reasons.

The point is that each tool I can recall, e.g., black and isort (see the issue), flake8 already supports it at least somehow.

I hear your point. I can only agree to disagree. I see people need the same thing as I do, so this PR solves the issue that has been hanging around for three years.

What I can do more here is write tests to put to stone the proposed behavior with pyproject.toml or .git or .hg in a directory above. I wouldn't invest my time to invest then even more time into wrapping around workarounds.

update: tests are added in e24f7d1

[Felixoid]

It looks like now .pylintrc is not parsed unless __init__.py is found in the version 2.15.5 after #7423, and it breaks behavior for ClickHouse/ClickHouse. So, in the recent versions neither https://github.com/ClickHouse/ClickHouse/blob/a3e3be5/.pylintrc works nor pyproject.toml could be set up in the root directory.

Not sure if it was intended, but the current state is a quite strange from the configuration perspective.

[Pierre-Sassoulas]

Sorry for the delay again @Felixoid. The 3.0 release cycle is coming, are you still available to work on this or should we take over ?

[Felixoid]

Thanks, I've rebased the changes, tests work locally

[codecov]

Codecov Report

Merging #7163 (a5aaf36) into main (07127ee) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7163   +/-   ##
=======================================
  Coverage   95.81%   95.81%           
=======================================
  Files         174      174           
  Lines       18334    18335    +1     
=======================================
+ Hits        17567    17568    +1     
  Misses        767      767           

Impacted Files	Coverage Δ
pylint/config/find_default_config_files.py	91.30% <100.00%> (+1.83%)	⬆️

... and 46 files with indirect coverage changes

[github-actions]

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 49de7a9

[Felixoid]

Looks green as a coming spring!

[Felixoid]

Hello, are there any updates on it?..

[DanielNoord]

I think we can merge this, but defer to @Pierre-Sassoulas for doing so!

[Pierre-Sassoulas]

Sorry for taking a long time to review and merge, this is going to be in the first beta version 3.0.0b1