Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove deprecations scheduled for 0.19 #5630

Merged
merged 13 commits into from
Jul 23, 2021
Merged

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Jul 22, 2021

As discussed in the meeting yesterday, here's the PR for the scheduled deprecations. We also have quite a few deprecations which are not scheduled... we should check which of them can be removed / completed, but that can wait until after the release.

I tried to keep the changes for each individual deprecation in their own commit, which should make rolling back easier in case we decide removing / completing a specific deprecation is too aggressive.

whats-new.rst still needs to be updated, but I'll leave that until after the code changes have been approved.

  • Passes pre-commit run --all-files
  • User visible changes (including notable bug fixes) are documented in whats-new.rst

@pep8speaks

This comment has been minimized.

@max-sixty
Copy link
Collaborator

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?)

@keewis keewis force-pushed the deprecations branch 2 times, most recently from ac97a75 to 48b20ef Compare July 22, 2021 23:43
@github-actions
Copy link
Contributor

github-actions bot commented Jul 23, 2021

Unit Test Results

         6 files  ±0           6 suites  ±0   53m 18s ⏱️ ±0s
16 200 tests ±0  14 481 ✔️ ±0  1 719 💤 ±0  0 ❌ ±0 
90 396 runs  ±0  82 236 ✔️ ±0  8 160 💤 ±0  0 ❌ ±0 

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

♻️ This comment has been updated with latest results.

xarray/core/dataset.py Outdated Show resolved Hide resolved
xarray/tests/test_dask.py Show resolved Hide resolved
@TomNicholas
Copy link
Member

TomNicholas commented Jul 23, 2021

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.

@mathause
Copy link
Collaborator

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

doc/whats-new.rst Outdated Show resolved Hide resolved
xarray/core/combine.py Show resolved Hide resolved
@keewis
Copy link
Collaborator Author

keewis commented Jul 23, 2021

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

@keewis
Copy link
Collaborator Author

keewis commented Jul 23, 2021

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.

@shoyer
Copy link
Member

shoyer commented Jul 23, 2021

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:

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.

@keewis
Copy link
Collaborator Author

keewis commented Jul 23, 2021

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.

@keewis keewis requested a review from shoyer July 23, 2021 18:47
@keewis keewis added the plan to merge Final call for comments label Jul 23, 2021
@keewis
Copy link
Collaborator Author

keewis commented Jul 23, 2021

@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.

@TomNicholas TomNicholas merged commit c5530d5 into pydata:main Jul 23, 2021
@keewis keewis deleted the deprecations branch July 23, 2021 20:13
st-bender added a commit to st-bender/xarray that referenced this pull request Aug 10, 2021
* main: (31 commits)
  Refactor index vs. coordinate variable(s) (pydata#5636)
  pre-commit: autoupdate hook versions (pydata#5685)
  Flexible Indexes: Avoid len(index) in map_blocks (pydata#5670)
  Speed up _mapping_repr (pydata#5661)
  update the link to `scipy`'s intersphinx file (pydata#5665)
  Bump styfle/cancel-workflow-action from 0.9.0 to 0.9.1 (pydata#5663)
  pre-commit: autoupdate hook versions (pydata#5660)
  fix the binder environment (pydata#5650)
  Update api.rst (pydata#5639)
  Kwargs to rasterio open (pydata#5609)
  Bump codecov/codecov-action from 1 to 2.0.2 (pydata#5633)
  new blank whats-new for v0.19.1
  v0.19.0 release notes (pydata#5632)
  remove deprecations scheduled for 0.19 (pydata#5630)
  Make typing-extensions optional (pydata#5624)
  Plots get labels from pint arrays (pydata#5561)
  Add to_numpy() and as_numpy() methods (pydata#5568)
  pin fsspec (pydata#5627)
  pre-commit: autoupdate hook versions (pydata#5617)
  Add dataarray scatter with 3d support (pydata#4909)
  ...
dcherian added a commit to kmsquire/xarray that referenced this pull request Aug 11, 2021
* upstream/main: (31 commits)
  Refactor index vs. coordinate variable(s) (pydata#5636)
  pre-commit: autoupdate hook versions (pydata#5685)
  Flexible Indexes: Avoid len(index) in map_blocks (pydata#5670)
  Speed up _mapping_repr (pydata#5661)
  update the link to `scipy`'s intersphinx file (pydata#5665)
  Bump styfle/cancel-workflow-action from 0.9.0 to 0.9.1 (pydata#5663)
  pre-commit: autoupdate hook versions (pydata#5660)
  fix the binder environment (pydata#5650)
  Update api.rst (pydata#5639)
  Kwargs to rasterio open (pydata#5609)
  Bump codecov/codecov-action from 1 to 2.0.2 (pydata#5633)
  new blank whats-new for v0.19.1
  v0.19.0 release notes (pydata#5632)
  remove deprecations scheduled for 0.19 (pydata#5630)
  Make typing-extensions optional (pydata#5624)
  Plots get labels from pint arrays (pydata#5561)
  Add to_numpy() and as_numpy() methods (pydata#5568)
  pin fsspec (pydata#5627)
  pre-commit: autoupdate hook versions (pydata#5617)
  Add dataarray scatter with 3d support (pydata#4909)
  ...
dcherian added a commit to dcherian/xarray that referenced this pull request Aug 13, 2021
* upstream/main: (34 commits)
  Use same bool validator as other inputs (pydata#5703)
  conditionally disable bottleneck (pydata#5560)
  Refactor index vs. coordinate variable(s) (pydata#5636)
  pre-commit: autoupdate hook versions (pydata#5685)
  Flexible Indexes: Avoid len(index) in map_blocks (pydata#5670)
  Speed up _mapping_repr (pydata#5661)
  update the link to `scipy`'s intersphinx file (pydata#5665)
  Bump styfle/cancel-workflow-action from 0.9.0 to 0.9.1 (pydata#5663)
  pre-commit: autoupdate hook versions (pydata#5660)
  fix the binder environment (pydata#5650)
  Update api.rst (pydata#5639)
  Kwargs to rasterio open (pydata#5609)
  Bump codecov/codecov-action from 1 to 2.0.2 (pydata#5633)
  new blank whats-new for v0.19.1
  v0.19.0 release notes (pydata#5632)
  remove deprecations scheduled for 0.19 (pydata#5630)
  Make typing-extensions optional (pydata#5624)
  Plots get labels from pint arrays (pydata#5561)
  Add to_numpy() and as_numpy() methods (pydata#5568)
  pin fsspec (pydata#5627)
  ...
dcherian added a commit to dcherian/xarray that referenced this pull request Aug 13, 2021
* upstream/main: (307 commits)
  Use same bool validator as other inputs (pydata#5703)
  conditionally disable bottleneck (pydata#5560)
  Refactor index vs. coordinate variable(s) (pydata#5636)
  pre-commit: autoupdate hook versions (pydata#5685)
  Flexible Indexes: Avoid len(index) in map_blocks (pydata#5670)
  Speed up _mapping_repr (pydata#5661)
  update the link to `scipy`'s intersphinx file (pydata#5665)
  Bump styfle/cancel-workflow-action from 0.9.0 to 0.9.1 (pydata#5663)
  pre-commit: autoupdate hook versions (pydata#5660)
  fix the binder environment (pydata#5650)
  Update api.rst (pydata#5639)
  Kwargs to rasterio open (pydata#5609)
  Bump codecov/codecov-action from 1 to 2.0.2 (pydata#5633)
  new blank whats-new for v0.19.1
  v0.19.0 release notes (pydata#5632)
  remove deprecations scheduled for 0.19 (pydata#5630)
  Make typing-extensions optional (pydata#5624)
  Plots get labels from pint arrays (pydata#5561)
  Add to_numpy() and as_numpy() methods (pydata#5568)
  pin fsspec (pydata#5627)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plan to merge Final call for comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants