Replace Python2 type comments with type annotations#870
Replace Python2 type comments with type annotations#870jklaise merged 2 commits intoSeldonIO:masterfrom
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #870 +/- ##
==========================================
+ Coverage 85.28% 85.29% +0.01%
==========================================
Files 74 74
Lines 8759 8768 +9
==========================================
+ Hits 7470 7479 +9
Misses 1289 1289
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
|
||
| sample_stats, pos, total = [], (), () # type: List, Tuple, Tuple | ||
| sample_stats: List = [] | ||
| pos: Tuple = tuple() |
There was a problem hiding this comment.
Is the tuple() preferred to ()?
There was a problem hiding this comment.
Just noticed we tend to mostly do tuple() elsewhere...
There was a problem hiding this comment.
I tend to use tuple because () is ambiguous and can create errors in certain contexts.
| # define head, tail part of the text | ||
| self.head, self.tail = '', '' # type: str, str | ||
| self.head_tokens, self.tail_tokens = [], [] # type: List[str], List[str] | ||
| self.head: str = '' |
There was a problem hiding this comment.
Since this crops up in a few places, we probably need to decide on a convention for handling these single-line multi-definition bits with type annotations. We could do something like:
self.head: str = ""; self.tail: str = ""There was a problem hiding this comment.
I'm happy with the above. This PEP suggest placing the type hints before the multiple assignment like so:
self.head: str
self.tail: str
self.head, self.tail = "", ""There was a problem hiding this comment.
I sort of think what you posted above is more readable. Although it doesn't save as many lines and I still don't love it!
There was a problem hiding this comment.
Yeah, I prefer self.head: str = ""; self.tail: str = "" tbh.
There was a problem hiding this comment.
Didn't seem to be very important, separate lines is nice as corresponds closely to how you might define types in a dataclass.
alibi/explainers/cfproto.py
Outdated
|
|
||
| # combine abdm and mvdm | ||
| self.d_abs = {} # type: Dict | ||
| self.d_abs: Any = {} |
There was a problem hiding this comment.
Why the change from Dict to Any?
There was a problem hiding this comment.
A mistake! Good catch! Thanks
alibi/utils/distance.py
Outdated
|
|
||
| d_pair = {} # type: Dict | ||
| X_cat_eq = {} # type: Dict | ||
| d_pair: dict = {} |
There was a problem hiding this comment.
Just noting that elsewhere we've done dict -> Dict but here it is Dict -> dict
ascillitoe
left a comment
ascillitoe
left a comment
There was a problem hiding this comment.
A few comments but generally LGTM!
3 participants
What is this:
Removes python2 type comments with type annotations, fixes #863. Required before we can merge #825.