issue_comments

Unit Test Results

6 files  ±0           6 suites  ±0   53m 18s :stopwatch: ±0s 16 200 tests ±0  14 481 :heavy_check_mark: ±0  1 719 :zzz: ±0  0 :x: ±0  90 396 runs  ±0  82 236 :heavy_check_mark: ±0  8 160 :zzz: ±0  0 :x: ±0 

Results for commit c5530d52. ± Comparison against base commit c5530d52.

:recycle: This comment has been updated with latest results.

@TomNicholas, since the latest commit reverted the completion of the .update deprecation cycle, I think it should be fine to merge this and issue the release.

Hello @keewis! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file xarray/tests/test_coarsen.py:

Line 16:1: F401 '.test_dataarray.da' imported but unused Line 17:1: F401 '.test_dataset.ds' imported but unused Line 20:36: F811 redefinition of unused 'ds' from line 17 Line 27:26: F811 redefinition of unused 'ds' from line 17 Line 42:25: F811 redefinition of unused 'ds' from line 17 Line 58:5: F811 redefinition of unused 'da' from line 16 Line 69:5: F811 redefinition of unused 'da' from line 16 Line 91:5: F811 redefinition of unused 'ds' from line 17 Line 160:25: F811 redefinition of unused 'ds' from line 17 Line 191:5: F811 redefinition of unused 'da' from line 16 Line 240:28: F811 redefinition of unused 'da' from line 16 Line 256:5: F811 redefinition of unused 'ds' from line 17

Comment last updated at 2021-07-23 18:46:56 UTC

this would essentially mean that we are extending the deprecation (since code and docs should be in sync)... we did consider the wrapper object in #4932, but I think back then we decided that warning in-code was too much trouble. I'll undo the removal and push the version to 0.21, which should give us some time to discuss what to do.

I would still return something from Dataset.update() for now.

I agree that it would be great to remove it, but to do that perhaps we could make some of Dataset subclass that issues a warning when any method is called on it? Perhaps something like: ```python import xarray import warnings

class DeprecatedDatasetFromUpdate(xarray.Dataset): slots = ['_accessed']

def init(self, args, kwargs): self._accessed = False super().init(args, **kwargs)

def getattribute(self, key): if not super().getattribute('_accessed'): warnings.warn( "This xarray.Dataset was created as the return value of " "xarray.Dataset.update(), but update() will return None in the " "future", category=FutureWarning) self._accessed = True return super().getattribute(key) ```

But could be a little tricky to get right, so perhaps let's save this for a later PR.

As far as I'm concerned this should be ready for a final review. We didn't yet decide what to do about the Dataset.update return value, though.

thanks for the whats-new.rst entries, @TomNicholas

Thanks @keewis for the keep_attrs deprecations (and for the other as well, of course).

This is great - there is one more to remove in combine.py isn't there? line 638

EDIT: I pushed a commit to this branch removing that one too.

Excellent, thanks a lot @keewis !

Maybe it's worth merging from the command line to retain the distinct commits on main (though maybe GH saves them?)