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

More strictness in mypy (WIP) #61

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

More strictness in mypy (WIP) #61

wants to merge 10 commits into from

Conversation

prophile
Copy link
Contributor

No description provided.

@prophile prophile requested review from PeterJCLaw and removed request for PeterJCLaw April 25, 2018 14:40
@@ -8,6 +8,8 @@ exclude=tests,migrations
[mypy]
ignore_missing_imports=true
strict_optional=true
disallow_any_generics=true
Copy link
Contributor

@mthpower mthpower Apr 25, 2018

Choose a reason for hiding this comment

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

What is an example of what this would disallow? (the documentation isn't very clear on this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

foo: List rather than foo: List[int]

@PeterJCLaw
Copy link
Contributor

👍

@coveralls
Copy link

coveralls commented May 19, 2018

Coverage Status

Coverage increased (+0.0001%) to 99.954% when pulling 49c12ef on mypy-strictness into 507f3d7 on master.

@danpalmer
Copy link
Contributor

@prophile I've fixed a few things here, but would appreciate a review as my approach might not be the best.

@prophile
Copy link
Contributor Author

I don't think we should bypass the layering tests for type checking. Actually I'd claim that the layering tests are correctly exposing dependencies here: where we're implicitly depending on the shape of component A from component B that's still a dependency on A from B even if it's previously not been unmasked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants