Wow, this is great stuff!

What's rs.randn()?

When this makes it into the public facing API it would be nice to include some guidance on how the chunking scheme affects the run time. Imagine a plot with run time plotted as a function of chunk size or number of chunks. Of course it also depends on the data size and the number of cores available.

To say it in a different way, array1.chunk({'place': 10}) is a performance tuning parameter, semantically no different than array1.

More ambitiously I could imagine an API such as array1.chunk('place') or array1.chunk('auto') meaning to figure out a reasonable chunking scheme only once .compute() is called so that all the compute steps are known. Maybe this is more specific to dask than xarray. I believe it would also be difficult.

This has been sitting for a little while... @shoyer please don't let my comment keep you from merging- it was more to note that this introduces custom error types.

I'm thinking about some of the ones in alignment.py which are currently ValueError, for example: https://github.com/pydata/xarray/blob/master/xarray/core/alignment.py#L303

Does it seem useful to you?

Yes, it's a nice sanity check.

Does the name join='exact' make sense?

Yes.

Any thoughts on this feature?

Is it worth introducing custom exception types into xarray? Seems like ValueError would be fine here. Then users can write this without having to know we use our own special exceptions: try: xarray.merge([ds1, ds2], join='exact') except ValueError: ...

Otherwise you'll probably want to change some others to AlignmentError for consistency.

Other datatypes would be extremely useful. But I think it would be better to start as a separate project and build some confidence in a system first.

@MaximilianR I was just typing nearly the same thing... :+1:

we might consider lightly wrapping NumPy arrays in a new object that also includes extra dtype information

Pandas seems to be moving away from this approach now.

Any other existing alternatives? datashape?

It is consistent with mpl, as long as neither vmin or vmax are set.

The reason for having consistency with MPL is that users can take essentially the same code and produce the same plots with or without xarray. And it's less work on this end.

We're departing from that here to get something that perhaps is more natural for the user. But I think it's ok since the plots generally won't look all that different. If users want more control they can just set all the levels explicitly.

I haven't used these arguments in the constructors. But I don't find them distracting either.

As an end user, it would be really nice to not have to worry about chunks at all. I'd like to write the same code in xarray using Numpy and have it do the right thing in dask transparently.

It seems like dask is moving in this direction (see Automatic blocksize for read_csv dask/dask#1147).

Agree with @shoyer that these features belong in dask.

Ah, now I see what you were going for. More going on here than I realized. That's a nice plot :)

The problem is with the shape of these coordinates.

```

ds = xr.tutorial.load_dataset('RASM_example_data') ds['xc'].shape (205, 275) ```

EDIT: just to be clear, it doesn't make sense to pass in 2d arrays for both x and y coordinates for a 2d plotting function.

Run this: ds.Tair[0].plot.pcolormesh(x='x', y='y') to produce:

The right thing for xarray to do is probably to throw an error when any 2d plot method is called with 2 coordinates that actually have higher dimensions.

Thanks for keeping me in the loop- hopefully next month I can participate. New baby and Phd quals => no time now

1 appears to be the simpler option off the top of my head.

@jhamman Seems like that would work. Might write a test to make sure the edges don't get dropped. I'd say let cartopy do as much as possible! cartopy integration was a lower priority when I was working on this, but only because we wanted to get the base stuff working first. More seamless integration or documentation for how to use with cartopy would be a good thing.

Wow, going for it! Great. Given the other common imports are

import numpy as np import pandas as pd import matplotlib.pyplot as plt

My vote goes for

import xarray as xr

Just curious, is xarray already decided on? You could call it namedarray or labelarray...

This is surprising! But good to know.

Yes, thanks.

But the docs on master are good? @shoyer or did you mean something about old vs new behavior?

Some changes have been made to the plot functions. This one works for me:

atlantic.plot(ax=ax, transform=ccrs.PlateCarree())

The latest version of the docs has a better plot that actually uses real data: http://xray.readthedocs.org/en/latest/plotting.html#maps

@fmaussion Can you post the contents of test_xray.py? I tried running a Cartopy script with

last conda install (xray 0.6.1, linux, python 3.4, mpl 1.4.3)

and everything looks good locally.

thanks @jhamman for making it work!

I think it's fine to have yincrease=True and xincrease=True everywhere as the defaults. This is how most people expect graphs to look. This is a place where internal consistency within xray probably beats conforming to the matplotlib defaults.

If the dimensions are A, B, C, and the call is da.plot.pcolormesh(col='B') then it seemed a little ambiguous to assign x='A', y='C'. So another option would be to raise an exception here.

Simplification of the logic is a good thing, so I'm fine with this.

We should call out the performance difference in the docs, maybe both in the main 2d plot section and the faceting section.

With a 1000 x 1000 grid I'm seeing 10 ms for imshow compared to 200 ms for pcolormesh. And much longer to render.

pcolormesh is more general, and works fine.

The problem, however, is time! How long does it take to plot and render with the two?

imshow can be orders of magnitude faster to plot and render.

Are the coordinates not dimensions of those DataArrays? Would it be better to make them dimensions when they get read in?

This looks good. The general strategy with the FacetGrid was to bring the minimum amount of code in from Seaborn to make it do what we wanted. I see no issues in iterating like this, ie. bring in pieces of the code with tests as we need them.

Side note- I'm back in grad school studying for qualifying exams, so forgive me if it takes me a while to respond in the future.

thanks for looking into it!

Looks good. You might consider testing this in Common2dMixin for better coverage.

But do the xray objects have to exist in memory? I was thinking this could also work along with open_mfdataset. It just loads and operates on the chunk it needs.

Like the idea of applying this to groupby objects. I wonder if it could be done transparently to the user...

Unfortunately this didn't seem to fix #562 http://xray.readthedocs.org/en/latest/plotting.html#other-features

Now we can finally start using A @ B! ;)

Dataset currently has around 100 attributes. It would be nice to keep this manageable.

update - docs are not building after some of the changes. Fixing now.

Wonder if this can also fix #562. The local doc build looks good.

Failing on Python 2.6 since there is no inspect.getcallargs. This particular test is important- it makes sure the 2d methods and the functions have the same signature. So it needs to skip if Python version is less than 2.7.

Not possible now. Wouldn't be much trouble to add it though. @drewokane has been working on generalizing the 1d plotting in #566. This might be a nice thing to include.

Still need tests for these. It would be possible to do something like test_plots.Common2dMixin here, or do something more direct.

Is this ready or do you want to wait until setting up cartopy?

Looks good. Do you know what's going on with the 3.4 tests?

On my local build I see this:

ds = xray.tutorial.load_dataset('air_temperature') percents = ds.groupby('time.month').reduce(np.percentile, q=5)

Producing:

In [80]: percents Out[80]: <xray.Dataset> Dimensions: (month: 12) Coordinates: * month (month) int64 1 2 3 4 5 6 7 8 9 10 11 12 Data variables: air (month) float64 239.7 239.3 244.3 251.7 261.4 271.5 274.0 272.5 ...

In the future we'll probably want to do more with Cartopy. So yes, it might be nice to get it going on Travis.

Ok, going to merge once the build passes. Will continue with the rest of the stuff next week. @shoyer should I rebase or squash these commits?

@shoyer Latest commit also added a little more to the docs on how to use the attributes data axes, and name_dicts.

By the way, I believe this is to a point where we could easily tune any of the aesthetics- say shrinking the width of the colorbar or using a different number of ticks / sizes on the labels.

Here's what's left to do: - [ ] convenience method darray.plot(row='z') - [ ] map example using matplotlib quiver plot - [ ] port any more methods / features from Seaborn FacetGrid

@shoyer, am I forgetting anything?

Ready for this PR to be reviewed now. I can finish the rest in a follow up PR.

For this PR: - [x] facet on rows and columns - [x] Align label on colorbar - [x] Label figure axes - [x] Use xray plotting logic to make nice colorbar - [x] Make and test choices around colorbar

EDIT - going to revisit plt.contour(color='k') after #537 gets worked out. - [x] separate iteration over data from plotting - edge case: require the index to actually be unique .is_unique. - [x] Use core string formatting for titles - [x] tests for all of above - [x] test col_wrap - [x] write and organize docs - [x] docs for public attribute- axes and such

On this PR or possibly a different one: - [ ] Convenience method to easily make faceted plots- something like darray.plot(col='time') - [x] port Seaborn's aspect capabilities - [ ] matplotlib quiver plot for map example - [x] Create new xray.tutorial module with function like load_example_dataset for plotting examples

Different PR: - [ ] Facets and maps - [ ] Inline documentation and plots for API reference

Works for me. And we still need to do imshow, right?

@shoyer multiple labels on x / y axes compared to the one above.

Starting to agree with @shoyer and Seaborn on the multiple labels:

Getting better...

Thanks for the encouragement @jhamman - I'll take what I can get! Seem to be finding a way around it by setting custom ax.xaxis.set_major_locator

Aaaaaaahhh!!

Issues seem to be mostly caused by the colorbar.

Just curious- how would we currently do this?

I believe it's possible to fix automatically. Somehow. Will look into it further tomorrow.

For reference, here's what is happening once aspect is adjusted. x axes are all crowded.

Some issues cropping up with using plt.tight_layout when manipulating the aspect and size.

take aspects and figure sizes from seaborn

Done

add ability to set margin_titles=True.

TODO- don't really understand the use case for this.

yep, could definitely do that. I'm still not crazy about repeating the x, y labels like that. Let me see if I can come up with a nice alternative using text on the figure.

Here's what we have now with one missing block.

Not sure about these aesthetics.

todo- make sure to seed random number generator or else use deterministic

Done

add matplotlib quiver plot for map example.

variable names must be hashable (and unique)

This makes datasets behave like dictionaries, which are familiar for Python users. A good thing.

raise an error in to_netcdf in the event the variable name is invalid.

@jhamman +1 for this solution

Here's where the progress stands:

g = xray.plot.FacetGrid(t5, col='time', col_wrap=2) g.map_dataarray(xray.plot.contour, color='k')

Looks like the column wrapping does the right thing by leaving the last axes blank.

Observe #537 is still an issue here. We'll have to make sure that when that is fixed it also works for these facets.

On Monday morning I can clean the code up, take out the seaborn chunks, reorganize the docs, and then I'll be ready to ask for a review.

@rabernat We were all Git noobs at some point- don't worry about it!

Seaborn should not be a dependency.

Merging

Were you planning on also adding these checks to the keys of a Dataset in this PR? Because I believe you can still do this:

c = xray.Dataset({'a': a, ('b', 'b'): b})

and it won't raise an error.

@jhamman any more feedback before merge?

@jhamman I took out the robust_percentile arg. You're right, it's better this way.

Was trying to keep the internal code a little more modular by parameterizing the percentiles.

But it was simpler before. Maybe should keep it like it was.

You mentioned in the bug report:

Another way is just to raise an error if a not string variable name is used.

Raising a TypeError seems like the way to go here, since str works on anything. Relevant line in 'Zen':

In the face of ambiguity, refuse the temptation to guess.

Also, are there other places to implement this? Say the keys of a dataset?

@shoyer has laid out a nice plan.

@rabernat do you feel like taking this on now? If not we can merge this and implement the rest later.

This seems useful. xray uses the netCDF4 library here, and they support it. In the meantime, you could always add a post processing step from the command line: http://www.unidata.ucar.edu/blogs/developer/en/entry/netcdf_compression.

I forgot to include requests as a dependency.

@rabernat Thanks!

Thanks for the report. This example indeed fails in 0.5.1, but it works in 0.5.2.

There's a fix documented in the latest release, but it doesn't reference a bug report- http://xray.readthedocs.org/en/stable/whats-new.html#bug-fixes

Closed by #466

nice- I had forgotten about those

For (2) I think it makes sense to extend the existing groupby to deal with multiple dimensions. Ie, let it take an iterable of dimension names.

```

darray.groupby(['lat', 'lon']) ```

Then we'd have something similar to the SQL groupby, which is a good thing.

By the way, in #527 we were considering using this approach to make the faceted plots on both rows and columns.

+1 This came up in discussion on #519

Thanks! The colorbar label spacing is a matplotlib thing- it only became a problem after rotating it to match the row labels. So I expect that I can fix it by changing the anchor point or placing it manually.

Here's what a 4d array looks like.

``` In [26]: t4d = xray.concat([t2, t2 + 50], pd.Index(['normal', 'hot'], name='fourth_dim'))

This is a 4d array

In [27]: t4d.coords Out[27]: Coordinates: * lat (lat) float32 75.0 72.5 70.0 67.5 65.0 62.5 60.0 57.5 55.0 ... * lon (lon) float32 200.0 202.5 205.0 207.5 210.0 212.5 215.0 ... * time (time) datetime64[ns] 2013-01-01 2013-03-04T12:00:00 * fourth_dim (fourth_dim) object 'normal' 'hot'

In [28]: g = xray.plot.FacetGrid(t4d, col='time', row='fourth_dim')

In [29]: g.map_dataarray(xray.plot.imshow, 'lon', 'lat') Out[29]: <xray.plot.facetgrid.FacetGrid at 0x10f5aad90> ```

@shoyer Here's what we currently have:

``` In [15]: g = xray.plot.FacetGrid(t, col='time', col_wrap=2)

In [16]: g.map_dataarray(xray.plot.contourf, 'lon', 'lat') Out[16]: <xray.plot.facetgrid.FacetGrid at 0x10de082d0> ```

Right now this assumes that the function for map_dataarray has the same signature as 2d xray plotting methods.

Maybe we should restrict squeezing the data to only the magic DataArray.plot() method?

Works for me. The error message should be informative enough in the other cases.

Scalar coordinates- this is the current behavior:

``` In [16]: a = xray.DataArray(np.random.randn(3, 4))

In [17]: a.coords['foo'] = 123

In [18]: a.plot() ```

So scalar coordinates do show up in the title, which is what I was going for. I could add a test for this.

labelling facets will be more reliable if it is done explicitly in the same code that is doing the indexing/groupby.

That's the approach that I've moved to after spending more time on it. This PR adds the argument add_labels to 2d plotting functions. Now for the faceting I'll actually use add_labels=False.

Will finish this up tomorrow morning.

@jhamman Yes, ready for review.

@jhamman good idea :+1:

``` In [1]: import matplotlib

In [2]: import matplotlib.pyplot

In [3]: matplotlib.use('Agg') /Users/clark.fitzgerald/anaconda/envs/xray_dev/lib/python2.7/site-packages/matplotlib/init.py:1318: UserWarning: This call to matplotlib.use() has no effect because the backend has already been chosen; matplotlib.use() must be called before pylab, matplotlib.pyplot, or matplotlib.backends is imported for the first time.

warnings.warn(_use_error_msg) ```

But really if people need to use a particular backend (like in our case) they should be setting it in the first place. Seaborn doesn't go beyond what pyplot does here, so I think it's ok.

Yes, this is a little tricky. Internally Seaborn uses lots of pyplot. And importing pyplot sets the backend. So maybe they could try something like pyplot.switch_backend within their reset_orig function?

Explanation-

Looks like the backend is set when Seaborn is imported. When building the docs we don't want to use an interactive backend.

``` (xray_dev)testimage ~/dev/xray/doc $ ipython Python 2.7.10 |Continuum Analytics, Inc.| (default, May 28 2015, 17:04:42)

...

In [1]: import seaborn

In [2]: import matplotlib

In [3]: matplotlib.use('Agg') /Users/clark.fitzgerald/anaconda/envs/xray_dev/lib/python2.7/site-packages/matplotlib/init.py:1318: UserWarning: This call to matplotlib.use() has no effect because the backend has already been chosen; matplotlib.use() must be called before pylab, matplotlib.pyplot, or matplotlib.backends is imported for the first time.

warnings.warn(_use_error_msg) ```

Yes, looks good. Merge away!

Nice work @jhamman @shoyer. Tests look pretty thorough and I like all the helper functions.

@kjordahl this is certainly an improvement.

We're planning on reworking many of these plotting examples to use this data, and it would be great to update this cartopy one as well.

Looks good. Merge?

I was thinking about only allowing it to work only if the array has exactly matching coordinates. Which would be the case in (4) a[(x > 0) & (y > 0)]. But then it would be difficult to stay consistent in the 1d case- mask or select?

My sense is that we'll probably be happier if we have entirely distinct APIs for masking (.where) and selection ([] and .loc[]).

That's a concrete and easy to understand distinction. I'm convinced.

go for it!