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

Mypy check untyped #60

Merged
merged 3 commits into from
Nov 7, 2016
Merged

Mypy check untyped #60

merged 3 commits into from
Nov 7, 2016

Conversation

lopuhin
Copy link
Contributor

@lopuhin lopuhin commented Nov 7, 2016

Enable checking untyped defs with mypy.

In order to do that, I had to add __init__ methods to base classes. To slightly avoid repetition, I removed explicit attr.id lines and instead add them based on __init__ signature - I don't see obvious downsides here or ways it could be misused (this is a bit magical, but attr.ib is also magical), but still that might cause problems or confusion - if so, we can roll back the attributes, while having __init__ methods too.

We need __init__ methods for type-checking, and extract attrs
from them to avoid repetition.
Add missing type annotations, fix a bug found by mypy,
enable --check-untyped-defs in tox.ini
This makes sure that we did not define extra or different
attributes in __init__.
@codecov-io
Copy link

codecov-io commented Nov 7, 2016

Current coverage is 95.98% (diff: 100%)

Merging #60 into master will increase coverage by 0.07%

@@             master        #60   diff @@
==========================================
  Files            26         27     +1   
  Lines          1221       1245    +24   
  Methods           0          0          
  Messages          0          0          
  Branches        201        205     +4   
==========================================
+ Hits           1171       1195    +24   
  Misses           26         26          
  Partials         24         24          
Diff Coverage File Path
•••••••••• 100% eli5/lime/samplers.py
•••••••••• 100% eli5/sklearn/text.py
•••••••••• 100% eli5/base.py
•••••••••• 100% eli5/sklearn/utils.py
•••••••••• 100% eli5/_feature_weights.py
•••••••••• 100% new eli5/base_utils.py
•••••••••• 100% eli5/sklearn/unhashing.py
•••••••••• 100% eli5/formatters/text.py

Powered by Codecov. Last update d7c8d45...6851e1f

@kmike
Copy link
Contributor

kmike commented Nov 7, 2016

See also: python-attrs/attrs#97

@lopuhin
Copy link
Contributor Author

lopuhin commented Nov 7, 2016

Thanks! Looks like python/mypy#1310 is a blocker for python-attrs/attrs#97, and the mypy issue looks very far away, sadly.

I though if this trick with generating attributes from __init__ could be useful for attrs, but it seems to be a bit too hacky to expose in such a base library.

@kmike
Copy link
Contributor

kmike commented Nov 7, 2016

Yeah, that mypy issue is a blocker. Also, it proposes a plugin architecture, but this means fix would be mypy-specific and other users (eg. PyCharm) won't benefit from that.

@kmike kmike merged commit 66bb7de into master Nov 7, 2016
@lopuhin lopuhin deleted the mypy-check-untyped branch November 7, 2016 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants