issue_comments: 457792344

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions performed_via_github_app issue
https://github.com/pydata/xarray/pull/2665#issuecomment-457792344 https://api.github.com/repos/pydata/xarray/issues/2665 457792344 MDEyOklzc3VlQ29tbWVudDQ1Nzc5MjM0NA== 6628425 2019-01-26T02:16:58Z 2019-01-26T02:16:58Z MEMBER

I think it would make sense to follow this example, but use cftime.datetime objects instead. You might want to add a test for a 2D plot just to be sure. https://github.com/pydata/xarray/blob/2e99c7dde32e396dd2c5fc6261a2d06217621e10/xarray/tests/test_plot.py#L1797-L1813 Note you'll also need to add nc-time-axis to some CI environments so that things run in some Travis/AppVeyor builds, probably best in: - ci/requirements-py37.yml - ci/requirements-py37-windows.yml

and add some decorators to skip the tests if the needed packages (cftime and nc-time-axis) are not installed. @requires_cftime already exists, but I think you'll have to write your own @requires_nc_time_axis decorator, which you can do, e.g., here: https://github.com/pydata/xarray/blob/2e99c7dde32e396dd2c5fc6261a2d06217621e10/xarray/tests/init.py#L65 Maybe by leaving nc-time-axis out of the py36 test environment (which has cftime) you can use it to test the error message if nc-time-axis is not installed?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   398041758