add mypy typing #203
add mypy typing #203
Conversation
| @@ -110,7 +111,7 @@ def get_lower_attrib(name): | |||
| return re.sub(r".*\.", "", name).lower() | |||
|
|
|||
|
|
|||
| class DublinCoreExtractor(object): | |||
There was a problem hiding this comment.
got rid of some python2 style stuff
| @@ -53,7 +56,7 @@ def lxmlDomNodeType(node): | |||
| return Node.NOTATION_NODE | |||
|
|
|||
|
|
|||
| class DomHtmlMixin(object): | |||
| class DomHtmlMixin: | |||
There was a problem hiding this comment.
mixins can be typed in python but it's a little verbose and doesn't seem key for this first pass
I think getting the top level user facing API properly typed is the primary goal of this PR
|
@BurnzZ when you get a chance |
| @@ -66,7 +65,6 @@ def get_version(): | |||
| "Natural Language :: English", | |||
| "Operating System :: OS Independent", | |||
| "Programming Language :: Python :: 3", | |||
| "Programming Language :: Python :: 3.6", | |||
There was a problem hiding this comment.
I'm not sure if we should drop 3.6 now. What are the missing annotations from 3.6 that you need? Are they available in https://pypi.org/project/typing-extensions/ ?
There was a problem hiding this comment.
the issue with python 3.6 is that it doesn't support from __future__ import annotations (which makes annotations lazily evaluated)
There was a problem hiding this comment.
@lopuhin do you foresee anything breaking in some of our stuff if we drop 3.6? I'm inclined to drop it here since it reached its EOL for almost a year now.
Moreover, I think it'd be great to formally support 3.10 and 3.11 while we're at it.
There was a problem hiding this comment.
@BurnzZ I'm +1 to drop python 3.6 since it's EOL, and support for 3.10 and 3.11 would be great too :)
| @@ -1,3 +1,4 @@ | |||
| # mypy: disallow_untyped_defs=False | |||
There was a problem hiding this comment.
Let's enforce typing in the tests as well.
One thing we've learned is that the test themselves test the type annotations of the package itself. For example: https://github.com/scrapinghub/web-poet/pull/78/files#diff-f46006e3f43ffb1dd5d6862005427f6620f4dcfb1fa2f883d8482550069eeecc
There was a problem hiding this comment.
yeah I agree, I think it can be a gradual thing, like first we type the top level, then we gradually add types for the rest of the package
|
@sbdchd I think we can merge this PR after adding a formal support for |
Adds basic support for #199
Only covers the top level functions
Also removes support for python 3.6 as that doesn't support
from __future__ import annotations