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

Fails `mypy --strict` because of reexports #3868

Closed
AMDmi3 opened this issue Jun 25, 2019 · 15 comments

Comments

@AMDmi3
Copy link
Contributor

commented Jun 25, 2019

Long story short

With python 3.7, fresh mypy 0.711:

import aiohttp
aiohttp.ClientSession
% mypy --strict test.py
test.py:3: error: Module has no attribute "ClientSession"

This happens because mypy --strict expects imported stuff reexported as in

from .client import BaseConnector as BaseConnector

while aiohttp/__init.py__ just imports stuff:

from .client import (BaseConnector, ...)

Related: python/mypy#7067

Expected behaviour

Clean mypy --strict, as these errors break checking code which uses aiohttp

@webknjaz

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

Feel free to send a PR :)

@AMDmi3

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2019

Would it be OK stylewise to just convert to a bunch of from .foo import bar as bar lines?

@webknjaz

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

That's a question to @asvetlov. I don't mind personally.

@asvetlov

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

Would it be OK stylewise to just convert to a bunch of from .foo import bar as bar lines?

Yes. Whatever works for you and isort tool :)

Please make sure that both __init__.py and web.py are processed.
[mypy] section from setup.cfg worth to be upgraded to support the most strict mode.
The problem is that mypy doesn't support strict=true in config files but requires decomposition of this mode into a bunch of separate flags. mypy is expanding, new options are added to the tool but we didn't change our config file yet.

AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jun 26, 2019
@AMDmi3

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2019

Pull requests are ready. I'm not touching other mypy stuff yet as more work is required there, and these changes seem to be sufficient to unbreak consumer code.

@AMDmi3

This comment has been minimized.

Copy link
Contributor Author

commented Jul 2, 2019

Ping?

@webknjaz

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

I'm deferring the merge to @asvetlov

AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jul 15, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jul 15, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jul 15, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jul 15, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jul 15, 2019
AMDmi3 added a commit to AMDmi3/aiohttp that referenced this issue Jul 15, 2019
@asvetlov

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

Fixed by #3872

@asvetlov asvetlov closed this Jul 19, 2019

@webknjaz

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

should we also make our CIs use strict mode?

@asvetlov

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

Make sense.
mypy changes a set of concrete options implied by --strict.
Maybe we have to use mypy --strict explicitly instead of a bunch of detailed options in a config file.

@webknjaz

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

I don't know why our CI didn't catch it then

@AMDmi3

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

Mypy version for CI is fixed at rather old 0.670 for aiohttp, while the latest version is 0.720 - that's why many issues are not caught. Bumping it would require fixing a handful of them first (I've tried, should be not too hard for someone more familiar with the code).

Regarding strict mode, it would be nice to enable it too, but note that it can't be set via setup.cfg (it's just not supported by mypy), so it's only possible to list all options which --strict enables explicitly. Here's what I use in setup.cfg:

[mypy]
#strict = True  # not supported
warn_unused_configs = True
disallow_subclassing_any = True
disallow_any_generics = True
disallow_untyped_calls = True
disallow_untyped_defs = True
disallow_incomplete_defs = True
check_untyped_defs = True
disallow_untyped_decorators = True
no_implicit_optional = True
warn_redundant_casts = True
warn_unused_ignores = True
warn_return_any = True
implicit_reexport = False

I also set warn_unreachable = True, it's new in 0.720 and not included into --strict, but I find it quite useful for it revealed a bunch of problems in my code. There are some false positives which can be fixed by adding explicit optional types. Example:

foo = None
...
foo = 1
if bar && foo:  # "right hand of && is always false"
   ...  # "dead code"

Fixed like this:

foo: Optional[int] = None
...
foo = 1
if bar && foo:
   ...
@webknjaz

This comment has been minimized.

Copy link
Member

commented Jul 22, 2019

@AMDmi3 I'd be glad to accept a series of PRs gradually fixing those mypy issues along with adding options and modifying mypy invocations in the test runner scripts.

@AMDmi3

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

Ok, I'll try to figure something out.

@AMDmi3

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

See #3927

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.