More annotations #3088
More annotations #3088
Conversation
| def set_coords(self, names, inplace=None): | ||
| def set_coords( | ||
| self, | ||
| names: 'Union[Hashable, Iterable[Hashable]]', |
There was a problem hiding this comment.
It would have been nicer to use Collection[Hashable], but Collection requires Python 3.6
There was a problem hiding this comment.
Note: Sequence is NOT ok, because sets are not sequences
| names = [names] | ||
| else: | ||
| names = list(names) |
There was a problem hiding this comment.
This forced casting could have been avoided with Collection :(
Codecov Report
@@ Coverage Diff @@
## master #3088 +/- ##
==========================================
- Coverage 92.68% 92.65% -0.04%
==========================================
Files 63 63
Lines 12773 12787 +14
Branches 2906 2906
==========================================
+ Hits 11839 11848 +9
- Misses 619 623 +4
- Partials 315 316 +1 |
xarray/core/dataset.py
Outdated
| def update(self, other, inplace=None): | ||
| def update( | ||
| self, | ||
| other: 'Union[Dataset, Mapping[Hashable, DataArray]]', |
There was a problem hiding this comment.
unsure about this - are there more options available for 'other'?
There was a problem hiding this comment.
This goes through the merge machinery (same as Dataset.__init__), so in principle I think you can put Variable objects or tuples of Variable arguments in the mapping values, too.
| @@ -161,13 +161,15 @@ class AttrAccessMixin: | |||
| _initialized = False | |||
|
|
|||
| @property | |||
| def _attr_sources(self): | |||
| """List of places to look-up items for attribute-style access""" | |||
| def _attr_sources(self) -> List[Mapping[Hashable, Any]]: | |||
There was a problem hiding this comment.
Is Mapping[Hashable, Any] identical to Mapping?
There was a problem hiding this comment.
There is no requirement for the keys of a Mapping to be hashable. This is relevant for the various setter methods Dataset.attrs which cast any Mapping to an OrderedDict.
|
Hello @crusaderky! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-07-09 15:59:46 UTC |
|
thanks @crusaderky ! |
A little incremental addition to type annotations. By no means complete, but it should be ready for merge in its own right nonetheless.