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

Added docs example for xarray.Dataset.get() #2894

Merged
merged 2 commits into from
Apr 16, 2019
Merged

Added docs example for xarray.Dataset.get() #2894

merged 2 commits into from
Apr 16, 2019

Conversation

jbusecke
Copy link
Contributor

Added example of xarray.Dataset.get method and alternative ds[var_list] syntax.

dcherian
dcherian reviewed Apr 15, 2019
View reviewed changes
doc/data-structures.rst Outdated
ds['temperature_double'] = (('x', 'y', 'time'), temp * 2)
ds['precipitation_double'] = (('x', 'y', 'time'), precip * 2)

list(ds.get(['temperature', 'temperature_double']))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above. I wasnt sure what to make of this either but kept if for consistency.

dcherian
dcherian reviewed Apr 15, 2019
View reviewed changes
doc/data-structures.rst Outdated
@@ -401,6 +401,20 @@ operations keep around coordinates:
list(ds[['x']])
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why there is a list here either :) but @jbusecke I think you just need to add ds[['x', 'temperature']] as an example here since the text already talks about passing a list of variable names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see. Honestly I find the ds[['x']] line quite confusing without context. Why should I chose one of the dimensions?
Should I remove the list(...) calls alltoghether?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the list calls need to be removed.

And I think changing ds[['x']] to ds[['x', 'temperature']] is probably all that is needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh the .get example is great too, let's keep that.

@shoyer
Copy link
Member

shoyer commented Apr 15, 2019

For context: we didn't explicitly implement a .get() method on Dataset, we get it implemented for free because xarray.Dataset inherits from collections.abc.Mapping. So the only practical difference between .get() and [] on Dataset is the same as the difference for a general Python mapping: .get() returns a default value (which defaults to None) instead of raising KeyError.

I'm not sure it's particularly useful to show examples using .get() here, unless we want to show this difference in behavior if a key is not found.

@jbusecke
Copy link
Contributor Author

Oh I see. It returns None if any of the keys is not found. That might indeed lead to confusion.
So should I just add an example with multiple variables using ds[['var1', 'var2']]?

@shoyer
Copy link
Member

shoyer commented Apr 15, 2019

So should I just add an example with multiple variables using ds[['var1', 'var2']]?

I think this is probably a better idea.

To be honest, I don't know if .get() if useful for anyone with a list -- it's implementation here was really an accident (based on how Mapping.get works) rather than an intentional choice.

@jbusecke
Copy link
Contributor Author

jbusecke commented Apr 15, 2019 •
edited
Loading

I am still a bit puzzled over the ds[['x', 'temperature']] suggestion. Maybe I am not getting something, but x is a dimension and the output of ds[['x', 'temperature']] is the same as ds[['temperature']]. I think it would be clearer to add a third variable (maybe in the code block above) and then select two out of three data variables?

@shoyer
Copy link
Member

shoyer commented Apr 15, 2019

Yes, I think it would make more sense to add an example with another variable.

@jbusecke
Copy link
Contributor Author

How about this one?

@dcherian
Copy link
Contributor

I missed that x is a coordinate.

Thanks @jbusecke

@dcherian dcherian merged commit 08942c2 into pydata:master Apr 16, 2019
dcherian added a commit to yohai/xarray that referenced this pull request Apr 19, 2019
@dcherian
Merge branch 'master' into yohai-ds_scatter
41cca04 
* master: (29 commits)
  Handle the character array dim name  (pydata#2896)
  Partial fix for pydata#2841 to improve formatting. (pydata#2906)
  docs: Move quick overview one level up (pydata#2890)
  Manually specify chunks in open_zarr (pydata#2530)
  Minor improvement of docstring for Dataset (pydata#2904)
  Fix minor typos in docstrings (pydata#2903)
  Added docs example for `xarray.Dataset.get()` (pydata#2894)
  Bugfix for docs build instructions (pydata#2897)
  Return correct count for scalar datetime64 arrays (pydata#2892)
  Indexing with an empty array (pydata#2883)
  BUG: Fix pydata#2864 by adding the missing vrt parameters (pydata#2865)
  Reduce length of cftime resample tests (pydata#2879)
  WIP: type annotations (pydata#2877)
  decreased pytest verbosity (pydata#2881)
  Fix mypy typing error in cftime_offsets.py (pydata#2878)
  update links to https (pydata#2872)
  revert to 0.12.2 dev
  0.12.1 release
  Various fixes for explicit Dataset.indexes (pydata#2858)
  Fix minor typo in docstring (pydata#2860)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcherian dcherian dcherian left review comments

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

Successfully merging this pull request may close these issues.

drop all but specified data_variables/coordinates as a convenience function
3 participants
@jbusecke @shoyer @dcherian