-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Apply mypy-tests custom config to other mypy-based tests #13825
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
Apply mypy-tests custom config to other mypy-based tests #13825
Conversation
…s' of https://github.com/Avasam/typeshed into Apply-mypy-tests-custom-config-to-other-mypy-based-tests
lib/ts_utils/mypy.py
Outdated
@contextmanager | ||
def temporary_mypy_config_file( | ||
configurations: Iterable[MypyDistConf], | ||
) -> Generator[tempfile._TemporaryFileWrapper[str]]: # pyright: ignore[reportPrivateUsage] |
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.
Not a full review, but I'd prefer to keep the temporary file workaround logic separate from the actual logic for the temporary 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.
Yeah sure that makes sense. We may have to reuse temporary files elsewhere. It'd be nice if already separate.
The workaround used to only be done in tests/mypy_test.py
, but I'll extract it and put it somewhere in ts_lib
# The configuration section in the metadata file looks like the following, with multiple module sections possible | ||
# [mypy-tests] | ||
# [mypy-tests.yaml] | ||
# module_name = "yaml" | ||
# [mypy-tests.yaml.values] | ||
# disallow_incomplete_defs = true | ||
# disallow_untyped_defs = true | ||
|
||
|
||
def mypy_configuration_from_distribution(distribution: str) -> list[MypyDistConf]: |
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 this should be a docstring ?
# The configuration section in the metadata file looks like the following, with multiple module sections possible | |
# [mypy-tests] | |
# [mypy-tests.yaml] | |
# module_name = "yaml" | |
# [mypy-tests.yaml.values] | |
# disallow_incomplete_defs = true | |
# disallow_untyped_defs = true | |
def mypy_configuration_from_distribution(distribution: str) -> list[MypyDistConf]: | |
def mypy_configuration_from_distribution(distribution: str) -> list[MypyDistConf]: | |
""" | |
The configuration section in the metadata file looks like the following, with multiple module sections possible | |
```toml | |
[mypy-tests] | |
[mypy-tests.yaml] | |
module_name = "yaml" | |
[mypy-tests.yaml.values] | |
disallow_incomplete_defs = true | |
disallow_untyped_defs = true | |
``` | |
""" |
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.
I actually prefer this to be a comment. It doesn't really feel like something that's relevant to the API. That said, it would probably be better to document it in CONTRIBUTING
instead.
Extracted from #13627
Also relates to #12178 and #8403
We have the infra/tooling in place for per-stub mypy configuration. But those configs didn't extend to other tests, making it unusable since stubtest would still fail.
Easier with review with whitespace changes hidden.