Mypy typing + make API nice and unified #123
Conversation
nbgl
commented
Jul 10, 2018
nbgl
commented
- Mypy typing for everyone!
- Bring Hamming similarity API in line with Dice coefficient API.
anonlink/blocking.py
Outdated
| @@ -1,15 +1,17 @@ | |||
| from abc import ABCMeta, abstractmethod | |||
| from itertools import chain, product, repeat | |||
| from numbers import Real | |||
| from numbers import Integral, Real | |||
There was a problem hiding this comment.
Integral doesn't appear to be used
anonlink/blocking.py
Outdated
| @@ -21,21 +23,22 @@ def _evalf(__funcs : 'Iterable[Callable[..., _T]]', # https://github.com/python | |||
| return (f(*args, **kwargs) for f in __funcs) | |||
|
|
|||
|
|
|||
| class _AssociativeBinaryOp(metaclass=ABCMeta): | |||
| __slots__ = '_funcs' | |||
| _Record = TypeVar('_Record') | |||
There was a problem hiding this comment.
Why BlockingFunction but _Record? Shouldn't these be consistent? Shouldn't you import the _Record defined in typechecking.py?
Also I'd only put the TypeVar directly above a class declaration if it isn't used anywhere else - in this case _Record is used throughout the following classes so give it some whitespace to separate it from the first class.
There was a problem hiding this comment.
_Record is only there to parametrise the generic. It doesn’t make sense for other modules to import it. Similarly, _Record in typechecking.py is just there to parametrise generics. I don’t think it makes sense to import a parametrisation variable from elsewhere. But I’m open to discussion on this.
I’ve fixed the whitespace!
| assert len(clk1) == len(clk2) | ||
| assert len(clk1) != 0 | ||
| return sum(map(operator.eq, clk1, clk2)) / len(clk1) | ||
| return 1 - (clk1 ^ clk2).count() / len(clk1) |
There was a problem hiding this comment.
This kind of made sense to put there. I changed the type annotations to support only bitarrays instead of the more general Sequence[bool]. I thought the similarity code should reflect that.
* Blocking functions * Minor docstring * Types for Mypy * Hamming similarity * Refactor tests * Minor refactor * Require NumPy * Multiparty greedy solver * Candidate generation. Still missing some features * Some optimisations * More optimisation * Still optimising * Ready for PR * Hamish PR comments * Fix Python 3.5 bug * Serialization * Serialisation of candidate pairs! * Update typechecking. * Fix typing * Fix tests on big numbers * Oops. Minor. * Minor comment * Brian PR comments * Bug fix + more Pythonic type checking * Only test on >=Python3.6 * Travis does not yet have Python 3.7... * GPU 1 does not yet have Python 3.7 * Hamish PR comments * Revert release to Python 3.6 * I forgot to commit this before merging… (#119) * Candidate generation refactor (#122) * Candidate generation * Minor docstring * Typo in docstring * Hamish PR Comments * New API for Dice Coefficient (#121) * New API for Dice Coefficient * Docstrings * Minor PEP8 * Typing * Make tests pass… * Brian PR comments * Mypy typing + make API nice and unified (#123) * Type hints * Bring Hamming similarity in line with rest of API * Make solver tests pass * Ok, solving tests should finally work now... * Brian PR comments * Make typechecking.Record importable * Add dependency on mypy extensions * Blocking style (#125) * Blocking style * Minor commenting * Improve coverage on edge cases + misc. testing (#127) * Type hints * Bring Hamming similarity in line with rest of API * Make solver tests pass * Ok, solving tests should finally work now... * Edge cases and tests * Restore old similarity tests * Minor: I don't know how that typo got there * Make Dice similarity tests pass with new Clkhash * Minor: unused imports * Style and docstrings (#130) * Style and comments * More style and comments * Mypy * Minor: Fix link * Brian PR comments
