Skip to content

Cleanup (mostly documentation) #337

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

Merged
merged 4 commits into from
Feb 26, 2015
Merged

Cleanup (mostly documentation) #337

merged 4 commits into from
Feb 26, 2015

Conversation

shoyer
Copy link
Member

@shoyer shoyer commented Feb 26, 2015

No description provided.

@shoyer shoyer added this to the 0.4 milestone Feb 26, 2015
@shoyer shoyer changed the title Clean (mostly documentation) Cleanup (mostly documentation) Feb 26, 2015
@shoyer
Copy link
Member Author

shoyer commented Feb 26, 2015

CC @jhamman

It would be awesome if you could update your example to work on xray 0.4 before I do the final release (there will be a release candidate).

I think the main change effecting this example that is that 'time.season' now returns strings like 'DFJ', not integers. See http://xray.readthedocs.org/en/latest/whats-new.html for more details.

shoyer added a commit that referenced this pull request Feb 26, 2015
Cleanup (mostly documentation)
@shoyer shoyer merged commit 3eb015b into pydata:master Feb 26, 2015
@shoyer shoyer deleted the cleanup branch February 26, 2015 07:43
@jhamman
Copy link
Member

jhamman commented Feb 27, 2015

@shoyer: I just ran the example and it still works without any modifications.

A few comments/questions:

  • Should season be listed in the "virtual variables" here: http://xray.readthedocs.org/en/latest/data-structures.html
  • With the addition of the nan-safe functions, numpy is throwing a lot of warnings for things like "empty slices" (ocean grid cells in my example never have data). Its not a big deal to silence them but it is a bit annoying.
  • Since ds.groupby('time.season').mean('time') returns a Dataset with a Coordinates variable named time.season, ds.sel(time.season='JJA') doesn't work for Python syntax reasons. So, I don't know if I would change the syntax used in my example (selecting my position). I'm not keen on this constructor: ds.sel(**{'time.season':'JJA'}). I'm wondering if it would be better to name the coordinates returned from "virtual variable" operations without the "time." portion. Just a thought.

I will still get you an updated version of the example.

@shoyer
Copy link
Member Author

shoyer commented Feb 27, 2015

Should season be listed in the "virtual variables" here: http://xray.readthedocs.org/en/latest/data-structures.html

Good catch

With the addition of the nan-safe functions, numpy is throwing a lot of warnings for things like "empty slices" (ocean grid cells in my example never have data). Its not a big deal to silence them but it is a bit annoying.

To fix this, you can install bottleneck (which will also make these operations much faster). We could also look into silencing the warnings. I'll add a note to the documentation somewhere.

Since ds.groupby('time.season').mean('time') returns a Dataset with a Coordinates variable named time.season, ds.sel(time.season='JJA') doesn't work for Python syntax reasons. So, I don't know if I would change the syntax used in my example (selecting my position). I'm not keen on this constructor: ds.sel(**{'time.season':'JJA'}). I'm wondering if it would be better to name the coordinates returned from "virtual variable" operations without the "time." portion. Just a thought.

This one is trickier. Let me make a new issue.

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

Successfully merging this pull request may close these issues.

2 participants