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

Deprecation cycles to finish for xarray 0.13 #3280

Closed
9 tasks done
shoyer opened this issue Sep 4, 2019 · 9 comments · Fixed by #3314
Closed
9 tasks done

Deprecation cycles to finish for xarray 0.13 #3280

shoyer opened this issue Sep 4, 2019 · 9 comments · Fixed by #3314

Comments

@shoyer
Copy link
Member

shoyer commented Sep 4, 2019

Clean-ups we should definitely do:

  • remove deprecated options from xarray.concat (deprecated back in July 2015!):
    if dim is None:
    warnings.warn(
    "the `dim` argument to `concat` will be required "
    "in a future version of xarray; for now, setting it to "
    "the old default of 'concat_dim'",
    FutureWarning,
    stacklevel=2,
    )
    dim = "concat_dims"
    if indexers is not None: # pragma: no cover
    warnings.warn(
    "indexers has been renamed to positions; the alias "
    "will be removed in a future version of xarray",
    FutureWarning,
    stacklevel=2,
    )
    positions = indexers
    if mode is not None:
    raise ValueError(
    "`mode` is no longer a valid argument to "
    "xarray.concat; it has been split into the "
    "`data_vars` and `coords` arguments"
    )
    if concat_over is not None:
    raise ValueError(
    "`concat_over` is no longer a valid argument to "
    "xarray.concat; it has been split into the "
    "`data_vars` and `coords` arguments"
    )
    (edit by @max-sixty )
  • argument order in DataArray.to_dataset (also from July 2015)
    if dim is not None and dim not in self.dims:
    (edit by @max-sixty )
  • remove the warning back reindex with variables with different dimensions (from 2017). This could either be replaced by replacing dimensions like sel or by simply raising an error for now and leaving replacing dimensions for later: indexing with broadcasting #1639):
    if isinstance(indexer, DataArray) and indexer.dims != (dim,):
    warnings.warn(
    "Indexer has dimensions {:s} that are different "
    "from that to be indexed along {:s}. "
    "This will behave differently in the future.".format(
    str(indexer.dims), dim
    ),
    FutureWarning,
    stacklevel=3,
    )
    (edit by @max-sixty )
  • remove xarray.broadcast_array, deprecated back in 2016 in 52ee95f (edit by @max-sixty )
  • remove Variable.expand_dims (deprecated back in August 2017), whose implementation actually looks like it's already broken:

    xarray/xarray/core/variable.py

    Lines 1232 to 1237 in 41fecd8

    warnings.warn(
    "Variable.expand_dims is deprecated: use " "Variable.set_dims instead",
    DeprecationWarning,
    stacklevel=2,
    )
    return self.expand_dims(*args)
    (edit by @max-sixty )
  • stop supporting a list of colors in the cmap argument (dating back to at least v0.7.0):

    xarray/xarray/plot/utils.py

    Lines 737 to 745 in d089df3

    # we should not be getting a list of colors in cmap anymore
    # is there a better way to do this test?
    if isinstance(cmap, (list, tuple)):
    warnings.warn(
    "Specifying a list of colors in cmap is deprecated. "
    "Use colors keyword instead.",
    DeprecationWarning,
    stacklevel=3,
    )
    (edit by @max-sixty )
  • push the removal of the compat and encoding arguments from Dataset/DataArray back to 0.14. These were only deprecated 7 months ago in deprecate compat & encoding #2703. (edit by @max-sixty )

Clean-ups to consider:

@shoyer shoyer mentioned this issue Sep 4, 2019
@mathause
Copy link
Collaborator

mathause commented Sep 6, 2019

stop supporting a list of colors in the cmap argument

I am a bit worried about the 3rd last item on your list. I currently still rely on this behavior because the alternative does not work for me, see #3284.

@shoyer
Copy link
Member Author

shoyer commented Sep 6, 2019

stop supporting a list of colors in the cmap argument

I am a bit worried about the 3rd last item on your list. I currently still rely on this behavior because the alternative does not work for me, see #3284.

OK, we should put this off until the alternative works! But I do think we want to stop overloading cmap in an incompatible way.

@max-sixty max-sixty mentioned this issue Sep 8, 2019
3 tasks
@max-sixty
Copy link
Collaborator

@shoyer I edited your comment to check-off the ones in #3292, hope that's OK. (would be good to have a system from GH that didn't involve free editing of others' comments!)

@max-sixty
Copy link
Collaborator

I think all the 'must-dos' are done

@dcherian
Copy link
Contributor

dcherian commented Sep 9, 2019

stop supporting a list of colors in the cmap argument

I am a bit worried about the 3rd last item on your list. I currently still rely on this behavior because the alternative does not work for me, see #3284.

@mathause Does your PR mean that we can go forward with the deprecation then?

@mathause
Copy link
Collaborator

mathause commented Sep 9, 2019

Yes, fine for me.

@dcherian
Copy link
Contributor

We were also planing to deprecate auto_combine for 0.13 but we've been warning about that only since 29 June, 2019 so that should be pushed to 0.14. I've added this to the list.

@shoyer
Copy link
Member Author

shoyer commented Sep 13, 2019 via email

@dcherian
Copy link
Contributor

#3313 and #3314 should be looked over by someone who knows the auto_combine and groupby code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants