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.

Sign up for GitHub

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

Jump to bottom

Allow swap_dims to take kwargs #4841

Merged
merged 1 commit into from
Jan 24, 2021
Merged

Allow swap_dims to take kwargs #4841

merged 1 commit into from
Jan 24, 2021

Conversation

max-sixty
Copy link
Collaborator

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

@max-sixty max-sixty merged commit d555172 into pydata:master Jan 24, 2021
@max-sixty max-sixty deleted the swap-dims-kwargs branch January 24, 2021 23:46
keewis
keewis reviewed Jan 25, 2021
View reviewed changes
Copy link
Collaborator

@keewis keewis left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need a few more changes to the docstrings, sorry for being a bit slow with the review. Also, we usually have the pattern of coords / coords_kwargs which makes dims_dict seem a bit out of place. Should we deprecate it in favor of dims? This currently has a conflict with this:

xarray/xarray/core/dataset.py

Line 3246 in a0c71c1

dims = tuple(dims_dict.get(dim, dim) for dim in v.dims)

but I think we can just call that variable_dims

xarray/core/dataset.py
@@ -3164,6 +3166,10 @@ def swap_dims(self, dims_dict: Mapping[Hashable, Hashable]) -> "Dataset":
Dictionary whose keys are current dimension names and whose values
are new names.

**dim_kwargs : {existing_dim: new_dim, ...}, optional
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**dim_kwargs : {existing_dim: new_dim, ...}, optional
**dims_kwargs : {existing_dim: new_dim, ...}, optional

xarray/core/dataarray.py
@@ -1708,6 +1710,10 @@ def swap_dims(self, dims_dict: Mapping[Hashable, Hashable]) -> "DataArray":
Dictionary whose keys are current dimension names and whose values
are new names.

**dim_kwargs : {dim: , ...}, optional
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**dim_kwargs : {dim: , ...}, optional
**dims_kwargs : {existing_dim: new_dim, ...}, optional

dcherian added a commit to TomNicholas/xarray that referenced this pull request Jan 29, 2021
@dcherian
Merge branch 'master' into integrate_dim_arg
170cc62 
* master:
  WIP: backend interface, now it uses subclassing  (pydata#4836)
  weighted: small improvements (pydata#4818)
  Update related-projects.rst (pydata#4844)
  iris update doc url (pydata#4845)
  Faster unstacking (pydata#4746)
  Allow swap_dims to take kwargs (pydata#4841)
  Move skip ci instructions to contributing guide (pydata#4829)
  fix issues in drop_sel and drop_isel (pydata#4828)
dcherian added a commit to dcherian/xarray that referenced this pull request Jan 29, 2021
@dcherian
Merge remote-tracking branch 'upstream/master' into dataset/quiver
e0f227f 
* upstream/master:
  speed up the repr for big MultiIndex objects (pydata#4846)
  dim -> coord in DataArray.integrate (pydata#3993)
  WIP: backend interface, now it uses subclassing  (pydata#4836)
  weighted: small improvements (pydata#4818)
  Update related-projects.rst (pydata#4844)
  iris update doc url (pydata#4845)
  Faster unstacking (pydata#4746)
  Allow swap_dims to take kwargs (pydata#4841)
  Move skip ci instructions to contributing guide (pydata#4829)
  fix issues in drop_sel and drop_isel (pydata#4828)
  Bugfix in list_engine (pydata#4811)
  Add drop_isel (pydata#4819)
  Fix RST.
  Remove the references to `_file_obj` outside low level code paths, change to `_close` (pydata#4809)
dcherian added a commit to dcherian/xarray that referenced this pull request Feb 3, 2021
@dcherian
Merge branch 'master' into groupby-plot
fd202df 
* master: (458 commits)
  Add units if "unit" is in the attrs. (pydata#4850)
  speed up the repr for big MultiIndex objects (pydata#4846)
  dim -> coord in DataArray.integrate (pydata#3993)
  WIP: backend interface, now it uses subclassing  (pydata#4836)
  weighted: small improvements (pydata#4818)
  Update related-projects.rst (pydata#4844)
  iris update doc url (pydata#4845)
  Faster unstacking (pydata#4746)
  Allow swap_dims to take kwargs (pydata#4841)
  Move skip ci instructions to contributing guide (pydata#4829)
  fix issues in drop_sel and drop_isel (pydata#4828)
  Bugfix in list_engine (pydata#4811)
  Add drop_isel (pydata#4819)
  Fix RST.
  Remove the references to `_file_obj` outside low level code paths, change to `_close` (pydata#4809)
  fix decode for scale/ offset list (pydata#4802)
  Expand user dir paths (~) in open_mfdataset and to_zarr. (pydata#4795)
  add a version info step to the upstream-dev CI (pydata#4815)
  fix the ci trigger action (pydata#4805)
  scatter plot by order of the first appearance of hue (pydata#4723)
  ...
@keewis keewis mentioned this pull request Feb 5, 2021
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keewis keewis keewis left review comments

@andersy005 andersy005 andersy005 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@max-sixty @andersy005 @keewis