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

Run mypy in CI with dependencies' type annotations #115

Merged
merged 6 commits into from
Dec 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ repos:
hooks:
- id: black
language_version: python3
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.740
hooks:
- id: mypy
- repo: https://gitlab.com/pycqa/flake8
rev: 3.7.9
hooks:
Expand Down
15 changes: 9 additions & 6 deletions environs.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ def method(
raise EnvSealedError("Env has already been sealed. New values cannot be parsed.")
missing = kwargs.pop("missing", None) or default
if isinstance(field_or_factory, type) and issubclass(field_or_factory, ma.fields.Field):
field = typing.cast(typing.Type[ma.fields.Field], field_or_factory)(missing=missing, **kwargs)
field = field_or_factory(missing=missing, **kwargs)
else:
field = typing.cast(FieldFactory, field_or_factory)(subcast=subcast, missing=missing, **kwargs)
field = field_or_factory(subcast=subcast, missing=missing, **kwargs)
parsed_key, raw_value, proxied_key = self._get_from_environ(name, ma.missing)
self._fields[parsed_key] = field
source_key = proxied_key or parsed_key
Expand Down Expand Up @@ -130,7 +130,7 @@ class Meta:
return type("", (schema_class,), attrs)


def _make_list_field(*, subcast: Subcast, **kwargs) -> ma.fields.List:
def _make_list_field(*, subcast: typing.Optional[type], **kwargs) -> ma.fields.List:
inner_field = ma.Schema.TYPE_MAPPING[subcast] if subcast else ma.fields.Field
return ma.fields.List(inner_field, **kwargs)

Expand Down Expand Up @@ -183,8 +183,8 @@ def _serialize(self, value: ParseResult, *args, **kwargs) -> str:

# Override deserialize rather than _deserialize because we need
# to call urlparse *after* validation has occurred
def deserialize(self, value: str, attr: str = None, data: typing.Mapping = None) -> ParseResult:
ret = super().deserialize(value, attr, data)
def deserialize(self, value: str, attr: str = None, data: typing.Mapping = None, **kwargs) -> ParseResult:
ret = super().deserialize(value, attr, data, **kwargs)
return urlparse(ret)


Expand All @@ -195,7 +195,10 @@ def _deserialize(self, value, *args, **kwargs) -> Path:


class LogLevelField(ma.fields.Int):
def _format_num(self, value) -> int:
# Type ignore, because the return type annotation of the super
# class is a private TypeVar incompatible with int. The annotation
# in super should probably be Any.
hukkin marked this conversation as resolved.
Show resolved Hide resolved
def _format_num(self, value) -> int: # type: ignore
try:
return super()._format_num(value)
except (TypeError, ValueError) as error:
Expand Down
3 changes: 3 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,6 @@ select = B,C,E,F,W,T4,B9

[mypy]
ignore_missing_imports = true
warn_unreachable = true
warn_unused_ignores = true
warn_redundant_casts = true
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
EXTRAS_REQUIRE = {
"django": DJANGO_REQUIRES,
"tests": ["pytest"] + DJANGO_REQUIRES,
"lint": ["flake8==3.7.9", "flake8-bugbear==19.8.0", "pre-commit~=1.20"],
hukkin marked this conversation as resolved.
Show resolved Hide resolved
"lint": ["flake8==3.7.9", "flake8-bugbear==19.8.0", "mypy==0.750", "pre-commit~=1.20"],
}
EXTRAS_REQUIRE["dev"] = EXTRAS_REQUIRE["tests"] + EXTRAS_REQUIRE["lint"] + ["tox"]
PYTHON_REQUIRES = ">=3.5"
Expand Down
9 changes: 6 additions & 3 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@ deps =
commands = pytest {posargs}

[testenv:lint]
deps = pre-commit~=1.20
skip_install = true
commands = pre-commit run --all-files
extras = lint
commands =
pre-commit run --all-files
; Run mypy outside pre-commit because pre-commit runs mypy in a venv
; that doesn't have dependencies or their type annotations installed.
mypy .
Copy link
Owner

Choose a reason for hiding this comment

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

Won't this mean that mypy runs twice--once in pre-commit and once here?

Copy link
Author

Choose a reason for hiding this comment

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

Yes it does. In pre-commit it doesn't have the typehints of dependencies' so the checks aren't as comprehensive. If you want, we can remove mypy from pre-commit. I left it because it can still be useful when making a git commit locally.

Copy link
Owner

Choose a reason for hiding this comment

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

I'd be fine removing mypy from pre-commit. the lint job is already the slowest job in CI at the moment, and I'd hate to make it slower. Also, it's one less thing to manually sync between pre-commit-config.yaml and setup.py.

Copy link
Owner

Choose a reason for hiding this comment

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

Also, maybe add a comment above this line explaining why we're running mypy outside of pre-commit

Copy link
Author

Choose a reason for hiding this comment

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

Removed mypy from pre-commit and made the comment in tox.ini


; Below tasks are for development only (not run in CI)

Expand Down