-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Walk up for all config files and handle precedence #18482
base: master
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
current_dir = os.path.abspath(os.getcwd()) | ||
|
||
while True: | ||
for name in defaults.CONFIG_NAMES + defaults.SHARED_CONFIG_NAMES: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that the files mypy.ini
and .mypy.ini
in the parent directories will work?
I don't think it's a good idea. It violates the rule of thumb that those files are used to work only from the current directory.
It's a too-wide change.
I suggest make a precedence for local files, then use pyproject.toml, and then use user-defined files.
This comment has been minimized.
This comment has been minimized.
USER_CONFIG_FILES: Final = ["~/.config/mypy/config", "~/.mypy.ini"] | ||
if os.environ.get("XDG_CONFIG_HOME"): | ||
USER_CONFIG_FILES.insert(0, os.path.join(os.environ["XDG_CONFIG_HOME"], "mypy/config")) | ||
|
||
CONFIG_FILES: Final = ( | ||
CONFIG_FILE + PYPROJECT_CONFIG_FILES + SHARED_CONFIG_FILES + USER_CONFIG_FILES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, simply change the order here to shared + pyproject
with changing the documentation? https://mypy.readthedocs.io/en/stable/command_line.html#config-file
Or, well, if you generally like the idea of having the config files searched up to the fs/project root, then it's the way to go, for sure 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain the changes, i.e. the new precedence rules, in more detail in the commit summary?
Yeah, I think the idea makes sense, thank you for advocating for it!
In #16965 (merged yesterday), we changed from only looking for a config file in the current directory to potentially walking up the directory tree (e.g. to git repo root). However, that PR only looks for pyproject.toml. I think it's easier for maintenance if the various config files are handled similarly, which is what this PR changes. This also fixes precedence: if you have both a mypy.ini and a pyproject.toml in the same directory, you will always get the mypy.ini, instead of inconsistent behaviour depending on if you're in a child directory or not. I'll update this PR with some docs. |
This comment has been minimized.
This comment has been minimized.
632471f
to
cb1f686
Compare
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Follow up to #16965
Fixes #16070
Handles other mypy configuration files and handles precedence between them. Also fixes few small things, like use in git worktrees