spencerahill / aospy

Python package for automated analysis and management of gridded climate data
Apache License 2.0
83 stars 12 forks source link

Modifications to follow the IRIDL conventions #296

Closed jdossgollin closed 5 years ago

jdossgollin commented 5 years ago

This PR would make the GRIDD_ATTRS more compatible with the IRIDL conventions, based on the Columbia IRI data library (http://iridl.ldeo.columbia.edu/). IRIDL conventions include X for longitude, Y for latitude, and T for time, which are the three changes made in this PR.

See http://iridl.ldeo.columbia.edu/SOURCES/.NOAA/.NCEP-NCAR/.CDAS-1/.DAILY/.Intrinsic/.PressureLevel/.temp/.

I would like to include P for pressure level for variables that use pressure coordinates, but this modification seems non-trivial.

spencerkclark commented 5 years ago

This looks great, thanks! The build failures (i.e. Appveyor and Travis) are all unrelated. To get things to pass for now, go ahead and add an xfail decorator to test_load_variable_does_not_warn in test_data_loader.py. For example:

@pytest.mark.xfail
@pytest.mark.parametrize(['start_date', 'end_date'],
                         _LOAD_VAR_DATE_RANGES.values(),
                         ids=list(_LOAD_VAR_DATE_RANGES.keys()))
def test_load_variable_does_not_warn(load_variable_data_loader,
                                     start_date, end_date):

With the latest released versions of pandas and xarray, there is currently an unrelated warning raised there:

aospy/test/test_data_loader.py::test_load_variable_does_not_warn[cftime]
  //anaconda/envs/aospy_dev/lib/python3.6/site-packages/xarray/coding/cftimeindex.py:160: FutureWarning: CFTimeIndex.data is deprecated and will be removed in a future version
    if self.data:

I fixed this upstream in xarray (pydata/xarray#2448), so it should no longer emit a warning once xarray version 0.11 is released, which should be soon.

spencerahill commented 5 years ago

C.f. https://github.com/spencerahill/aospy/issues/293#issuecomment-437542424, @jdossgollin I'd say all that's left here is adding P for pressure. And a What's New entry.

Edit: and nbnds for BOUNDS_STR, c.f. https://github.com/spencerahill/aospy/issues/299#issuecomment-437084968

jdossgollin commented 5 years ago

Note to self: see also discussion in #293 comments

spencerkclark commented 5 years ago

Also a quick note, what I wrote here, https://github.com/spencerahill/aospy/pull/296#issuecomment-435674393, is no longer needed (since xarray 0.11 is now out).