Is there anything else that I need to do at this point? Sorry for the xarray noob question...

Awesome. Just added the line. Let me know if you think it is appropriate.

Thanks. I updated the PR accordingly.

Seems like the travis builds all pass, wohoo. Please let me know if anything else is needed.

I think I have addressed all the above remarks (Many thanks for the thorough review and tips). Waiting for the CI again.

I am smh getting these errors in the backend part of the tests:

``` ======================================================== FAILURES ========================================================= _____ TestNetCDF3ViaNetCDF4Data.test_encoding_same_dtype ______

self = <xarray.tests.test_backends.TestNetCDF3ViaNetCDF4Data object at 0xd238d9a20>

def test_encoding_same_dtype(self):
    ds = Dataset({'x': ('y', np.arange(10.0, dtype='f4'))})
    kwargs = dict(encoding={'x': {'dtype': 'f4'}})
    with self.roundtrip(ds, save_kwargs=kwargs) as actual:

      assert actual.x.encoding['dtype'] == 'f4'

E AssertionError: assert dtype('>f4') == 'f4'

xarray/tests/test_backends.py:853: AssertionError _______ TestGenericNetCDFData.test_encoding_same_dtype ________

self = <xarray.tests.test_backends.TestGenericNetCDFData object at 0xd238e0588>

def test_encoding_same_dtype(self):
    ds = Dataset({'x': ('y', np.arange(10.0, dtype='f4'))})
    kwargs = dict(encoding={'x': {'dtype': 'f4'}})
    with self.roundtrip(ds, save_kwargs=kwargs) as actual:

      assert actual.x.encoding['dtype'] == 'f4'

E AssertionError: assert dtype('>f4') == 'f4'

xarray/tests/test_backends.py:853: AssertionError ``` They do not always show up...not sure what to make of it, but could be an issue with my local environment. Lets see if the CI shows this aswell.

Ok I think I have most of the things covered. All test pass for me locally. What should I add to the whats-new.rst. I thought of something like this under Enhancements (or would this be considered a bug fix?): Internal plotting now supports cftime.datetime objects as time axis (@spencerkclark, @jbusecke)

Great idea to simplify @spencerkclark. Thanks. Regarding the tests. I have removed the following:

``` @requires_cftime def test_plot_cftime_coordinate_error(): cftime = _import_cftime() time = cftime.num2date(np.arange(5), units='days since 0001-01-01', calendar='noleap') data = DataArray(np.arange(5), coords=[time], dims=['time']) with raises_regex(TypeError, 'requires coordinates to be numeric or dates'): data.plot()

@requires_cftime def test_plot_cftime_data_error(): cftime = _import_cftime() data = cftime.num2date(np.arange(5), units='days since 0001-01-01', calendar='noleap') data = DataArray(data, coords=[np.arange(5)], dims=['x']) with raises_regex(NotImplementedError, 'cftime.datetime'): data.plot() ``` And the test suite passes locally.

But I assume Ill have to add another test dataset with a cftime.datetime time-axis, which then gets dragged through all the plotting tests? Where would I have to put that in?

Many thanks for all the help

I have quickly looked into the testing and found an oddity that might be important if nc-time-axis is not installed.

So in the definition of plot in plot.py I have changed

if contains_cftime_datetimes(darray): to if any([contains_cftime_datetimes(darray[dim]) for dim in darray.dims]): Because if I understand correctly, the previous statement only checks the dtype of the actual data, not the dimensions. Is this appropriate or am I misunderstanding the syntax? In my example above it doesnt matter, because this only spits out an error message when nc-time-axis is not available.

Ok so the plotting works now with both timeseries and 2d data as follows

``` import xarray as xr import numpy as np %matplotlib inline

Create a simple line dataarray with cftime

time = xr.cftime_range(start='2000', periods=4, freq='1H', calendar='noleap') data = np.random.rand(len(time)) da = xr.DataArray(data, coords=[('time', time)]) da.plot() ![download](https://user-images.githubusercontent.com/14314623/51777752-6d427580-20cc-11e9-9c3d-a91d31b10312.png)

Check with 2d data

time = xr.cftime_range(start='2000', periods=6, freq='2MS', calendar='noleap') data2 = np.random.rand(len(time), 4) da2 = xr.DataArray(data2, coords=[('time', time), ('other', range(4))]) da2.plot() ```

Cool. Ill give it a shot right now.

Sounds good to me.

Best

Julius On Jan 23, 2019, 12:56 PM -0500, Spencer Clark notifications@github.com, wrote:

I agree @dcherian; I just pinged the PR again, but if there is no activity there by this time next week, I think we should probably move forward here. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

Is there still interest in this PR? Or did the upstream changes move ahead? I am finding myself explaining workarounds for this too students in the department, so maybe my time would be better invested getting this fix to the full community?

But obviously if things are going to be fixed upstream soon, I would devote time to other projects. Thoughts?

Oh shoot, I now remember seeing this. If this will be implemented soon I guess the PR can be discarded. Any chance you would have a quick solution for the pcolormesh plot error (second example in the PR) @spencerkclark?

One of the more general questions I had was if we should expose the conversion using nc-time-axis in the public API. That way users could easily plot the data in matplotlib, e.g.: da_new = da.convert_cftime() plt.plot(da_new.time, da_new) Just an idea...

I have been along the lines of a short example. This works for timeseries data.

``` import xarray as xr import numpy as np %matplotlib inline

Create a simple line dataarray with cftime

time = xr.cftime_range(start='2000', periods=6, freq='2MS', calendar='noleap') data = np.random.rand(len(time)) da = xr.DataArray(data, coords=[('time', time)]) da.plot() ```

For pcolormesh plots this still fails.

```

Create a simple line dataarray with cftime

time = xr.cftime_range(start='2000', periods=6, freq='2MS', calendar='noleap') data2 = np.random.rand(len(time), 4) da2 = xr.DataArray(data2, coords=[('time', time), ('other', range(4))]) da2.plot() ```

Perhaps @spencerkclark has an idea how to deal with differencing cftime.datetime objects?