More annotations in Dataset by crusaderky · Pull Request #3112 · pydata/xarray

crusaderky · 2019-07-13T19:06:49Z

No description provided.

crusaderky · 2019-07-13T19:08:02Z

-    from .dataarray import DataArray
-    from .dataset import Dataset
+    from .dataarray import DataArray  # noqa: F811
+    from .dataset import Dataset  # noqa: F811


This is a workaround to a flake8 limitation https://gitlab.com/pycqa/flake8/issues/553

crusaderky · 2019-07-13T19:13:01Z

+        elif isinstance(dim, Mapping):
+            # We're later going to modify dim in place; don't tamper with
+            # the input
+            dim = OrderedDict(dim)


Fixed bug where dim was modified in place, tainting the input variable. Also, fixed crash if the input dim is a read-only mapping.

codecov · 2019-07-13T19:17:00Z

Codecov Report

Merging #3112 into master will decrease coverage by 0.04%.
The diff coverage is 88.57%.

@@            Coverage Diff             @@
##           master    #3112      +/-   ##
==========================================
- Coverage   95.98%   95.94%   -0.05%     
==========================================
  Files          63       63              
  Lines       12802    12816      +14     
==========================================
+ Hits        12288    12296       +8     
- Misses        514      520       +6

crusaderky · 2019-07-14T19:49:06Z

+        append: bool = False,
+        inplace: bool = None,
+        **indexes_kwargs: Union[Hashable, Sequence[Hashable]]
+    ) -> 'Dataset':


Worth noting the behaviour is inconsistent with DataArray and pandas, which return None when inplace=True. Changing it however would benefit nobody and would break stuff.

crusaderky · 2019-07-14T20:05:41Z

@shoyer @max-sixty ready for review and merge

max-sixty

This looks great!

max-sixty · 2019-07-15T15:11:54Z

+        indexes: Mapping[Hashable, Union[Hashable, Sequence[Hashable]]] = None,
+        append: bool = False,
+        inplace: bool = None,
+        **indexes_kwargs: Union[Hashable, Sequence[Hashable]]


I'm guessing this is fine because mypy passes - my original thought was whether this should be Optional[Mapping] given it was either None or a dict?

As of mypy 0.701, x: MyType = None and x: Optional[MyType] = None are the same.

from typing import Optional def f1(x: int = None) -> None: pass def f2(x: Optional[int] = None) -> None: pass def f3(x: int = 1) -> None: pass def f4(x: Optional[int] = 1) -> None: pass f1() f2() f3() f4() f1(1) f2(1) f3(1) f4(1) f1(None) f2(None) f3(None) f4(None)

mypy output:

test_mypy.py:27: error: Argument 1 to "f3" has incompatible type "None"; expected "int"

@max-sixty I did a generalised cleanup of all already existing cases; I find it's much more readable now

max-sixty · 2019-07-15T15:13:50Z

+        self,
+        dims_or_levels: Union[Hashable, Sequence[Hashable]],
+        drop: bool = False,
+        inplace: bool = None,


inplace should go away soon so shouldn't matter... but should this be Optional[bool] given the default is None?

max-sixty · 2019-07-15T15:15:21Z

+        Variable,
+        Tuple[Hashable, Any],
+        Tuple[Sequence[Hashable], Any],
+    ]


pep8speaks · 2019-07-15T18:04:37Z

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-24 22:54:48 UTC

max-sixty · 2019-07-15T19:02:20Z

Great! Agree re Optional, thanks for clearing up.
Good to go from my end

crusaderky · 2019-07-24T09:20:35Z

@shoyer this is ready for merge unless there are comments

max-sixty · 2019-07-24T16:51:26Z

+    [mypy-xarray]
+    ignore_missing_imports = True
+
+   By `Guido Imperiale <https://github.com/crusaderky>`_


Let's move this up to 0.12.4?

max-sixty · 2019-07-24T16:53:39Z

Thanks @crusaderky ! I think fine to merge given we've both reviewed

shoyer · 2019-07-24T21:08:26Z

+1 please go ahead without me!

…

On Wed, Jul 24, 2019 at 9:53 AM Maximilian Roos ***@***.***> wrote: Thanks @crusaderky <https://github.com/crusaderky> ! I think fine to merge given we've both reviewed — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3112?email_source=notifications&email_token=AAJJFVQMF2SAWWTI66BFMGDQBCCJLA5CNFSM4IDA3ET2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2W6OCY#issuecomment-514713355>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJJFVR5RRGF4CGOGIDDU63QBCCJLANCNFSM4IDA3ETQ> .

crusaderky · 2019-07-31T16:21:33Z

@max-sixty @shoyer can we merge please?

max-sixty · 2019-07-31T17:47:56Z

Sorry @crusaderky - we were all waiting on each other! My mistake

max-sixty · 2019-07-31T17:48:19Z

Thanks again for the PR

* master: More annotations in Dataset (pydata#3112) Hotfix for case of combining identical non-monotonic coords (pydata#3151) changed url for rasterio network test (pydata#3162)

* master: (68 commits) enable sphinx.ext.napoleon (pydata#3180) remove type annotations from autodoc method signatures (pydata#3179) Fix regression: IndexVariable.copy(deep=True) casts dtype=U to object (pydata#3095) Fix distributed.Client.compute applied to DataArray (pydata#3173) More annotations in Dataset (pydata#3112) Hotfix for case of combining identical non-monotonic coords (pydata#3151) changed url for rasterio network test (pydata#3162) to_zarr(append_dim='dim0') doesn't need mode='a' (pydata#3123) BUG: fix+test groupby on empty DataArray raises StopIteration (pydata#3156) Temporarily remove pynio from py36 CI build (pydata#3157) missing 'about' field (pydata#3146) Fix h5py version printing (pydata#3145) Remove the matplotlib=3.0 constraint from py36.yml (pydata#3143) disable codecov comments (pydata#3140) Merge broadcast_like docstrings, analyze implementation problem (pydata#3130) Update whats-new for pydata#3125 and pydata#2334 (pydata#3135) Fix tests on big-endian systems (pydata#3125) XFAIL tests failing on ARM (pydata#2334) Add broadcast_like. (pydata#3086) Better docs and errors about expand_dims() view (pydata#3114) ...

crusaderky added 3 commits July 13, 2019 17:23

Let mypy discover xarray typing hints

5506eff

Docs tweak

a537fbb

More annotations in Dataset

3b13dcb

crusaderky commented Jul 13, 2019

View reviewed changes

crusaderky added 3 commits July 14, 2019 00:55

Merge remote-tracking branch 'upstream/master' into annotations

6851c2f

Fix Python 3.5-specific check for OrderedDict

29abedf

typing in merge

44cfb8e

crusaderky commented Jul 14, 2019

View reviewed changes

max-sixty approved these changes Jul 15, 2019

View reviewed changes

crusaderky added 2 commits July 15, 2019 18:59

Merge remote-tracking branch 'upstream/master' into annotations

b119ddc

broadcast_like typing tweaks

9f71e0c

crusaderky added 2 commits July 15, 2019 19:21

Changed redundant x: Optional[MyType] = None to x: MyType = None

829f5ff

flake8

298f88b

Merge remote-tracking branch 'upstream/master' into annotations

52114a9

crusaderky closed this Jul 19, 2019

crusaderky reopened this Jul 19, 2019

Merge remote-tracking branch 'upstream/master' into annotations

b0b6edc

crusaderky closed this Jul 20, 2019

crusaderky reopened this Jul 20, 2019

Merge remote-tracking branch 'upstream/master' into annotations

e0036e7

max-sixty reviewed Jul 24, 2019

View reviewed changes

What's New

7a36ff5

max-sixty merged commit 1757dff into pydata:master Jul 31, 2019

crusaderky deleted the annotations branch August 1, 2019 10:41

simonjayhawkins mentioned this pull request Nov 28, 2021

CI: xarray upgrade to 0.19 broke CI Checks pipeline pandas-dev/pandas#42716

Closed

Uh oh!

Conversation

crusaderky commented Jul 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jul 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crusaderky commented Jul 14, 2019

Uh oh!

max-sixty left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pep8speaks commented Jul 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2019-07-24 22:54:48 UTC

Uh oh!

max-sixty commented Jul 15, 2019

Uh oh!

crusaderky commented Jul 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

max-sixty commented Jul 24, 2019

Uh oh!

shoyer commented Jul 24, 2019 via email

Uh oh!

crusaderky commented Jul 31, 2019

Uh oh!

max-sixty commented Jul 31, 2019

Uh oh!

max-sixty commented Jul 31, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

crusaderky commented Jul 13, 2019 •

edited

Loading

codecov Bot commented Jul 13, 2019 •

edited

Loading

pep8speaks commented Jul 15, 2019 •

edited

Loading