Skip to content
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

Migrate mypy config to pyproject.toml #3936

Merged
merged 9 commits into from Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -4,6 +4,7 @@
_build
.DS_Store
.vscode
.python-version
docs/_static/pypi.svg
.tox
__pycache__
Expand Down
2 changes: 2 additions & 0 deletions .pre-commit-config.yaml
Expand Up @@ -43,6 +43,7 @@ repos:
hooks:
- id: mypy
exclude: ^docs/conf.py
args: ["--config", "pyproject.toml"]
additional_dependencies:
- types-PyYAML
- tomli >= 0.2.6, < 2.0.0
Expand All @@ -51,6 +52,7 @@ repos:
- platformdirs >= 2.1.0
- pytest
- hypothesis
- aiohttp >= 3.7.4

- repo: https://github.com/pre-commit/mirrors-prettier
rev: v3.0.3
Expand Down
46 changes: 0 additions & 46 deletions mypy.ini

This file was deleted.

23 changes: 23 additions & 0 deletions pyproject.toml
Expand Up @@ -223,3 +223,26 @@ omit = [
]
[tool.coverage.run]
relative_files = true

[tool.mypy]
# Specify the target platform details in config, so your developers are
# free to run mypy on Windows, Linux, or macOS and get consistent
# results.
python_version = "3.8"
mypy_path = "src"
strict = true
# Unreachable blocks have been an issue when compiling mypyc, let's try to avoid 'em in the first place.
warn_unreachable = true
implicit_reexport = true
show_error_codes = true
show_column_numbers = true

[[tool.mypy.overrides]]
module = ["pathspec.*", "IPython.*", "colorama.*", "tokenize_rt.*", "uvloop.*", "_black_version.*"]
ignore_missing_imports = true

# CI only checks src/, but in case users are running LSP or similar we explicitly ignore
# errors in test data files.
[[tool.mypy.overrides]]
module = ["tests.data.*", "scripts.*"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't ignore errors in scripts. It's fine to have a looser config apply to scripts though, like ignore missing imports

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I thought about it but then skipped as it'll add few extra ignores in repo.

  • Added type: ignore where necessary in scripts.*
  • Added types-commonmark, urllib3, hypothesmith as additional_dependencies in mypy hook to avoid adding type: ignore (let me know if its not the right way)

ignore_errors = true
2 changes: 1 addition & 1 deletion src/blackd/__init__.py
Expand Up @@ -74,7 +74,7 @@ def main(bind_host: str, bind_port: int) -> None:
app = make_app()
ver = black.__version__
black.out(f"blackd version {ver} listening on {bind_host} port {bind_port}")
web.run_app(app, host=bind_host, port=bind_port, handle_signals=True, print=None)
web.run_app(app, host=bind_host, port=bind_port, handle_signals=True, print=None) # type: ignore[arg-type]
Copy link
Collaborator

Choose a reason for hiding this comment

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

What was the issue here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

print argument accepts Callable[..., None] = print
i.e.

src/blackd/__init__.py:77:81: error: Argument "print" to "run_app" has incompatible type "None"; expected "Callable[..., None]"  [arg-type]

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like this is fixed on master: https://github.com/aio-libs/aiohttp/blob/30850babb43a8e28dd2df036776c62fd613d3d89/aiohttp/web.py#L453C5-L453C5. Can you add a comment saying that aiohttp had an incorrect annotation and that it will be fixed once aiohttp releases that code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, thanks



def make_app() -> web.Application:
Expand Down
2 changes: 1 addition & 1 deletion tests/optional.py
Expand Up @@ -26,7 +26,7 @@
from pytest import StashKey
except ImportError:
# pytest < 7
from _pytest.store import StoreKey as StashKey # type: ignore[no-redef]
from _pytest.store import StoreKey as StashKey # type: ignore[import, no-redef]

log = logging.getLogger(__name__)

Expand Down
2 changes: 1 addition & 1 deletion tests/test_blackd.py
Expand Up @@ -31,7 +31,7 @@ def unittest_run_loop(func, *args, **kwargs):


@pytest.mark.blackd
class BlackDTestCase(AioHTTPTestCase): # type: ignore[misc]
class BlackDTestCase(AioHTTPTestCase):
def test_blackd_main(self) -> None:
with patch("blackd.web.run_app"):
result = CliRunner().invoke(blackd.main, [])
Expand Down