search

COSIMA / cosima-recipes

A cookbook of recipes (i.e., examples) for analysing ocean and sea ice model output
https://cosima-recipes.readthedocs.io
Apache License 2.0
44 stars 66 forks source link

Intake conversion Equatorial_thermal_and_zonal_velocity_structure #359

Closed rbeucher closed 1 month ago

rbeucher commented 3 months ago

Following the discussion in issue #313, we propose converting the recipes to use Intake, given that the Cookbook is no longer supported and the ACCESS-NRI Intake catalog is now available.

A few months ago, @max-anu began working on this transition. This pull request contains the changes @max-anu made to the notebook specified in the title.

review-notebook-app[bot] commented 3 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

review-notebook-app[bot] commented 1 month ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-08-30T01:22:03Z ----------------------------------------------------------------

Line #6.        darray = catalog[expt].search(variable = 'temp', frequency = '1mon').to_dask(xarray_open_kwargs = {'use_cftime':True})

I think 'use_cftime':True is the default so you cam remove that.

This returns a dataset, so better to call it ds, or temp_ds


_julia-neme commented on 2024-08-30T03:42:27Z_ ----------------------------------------------------------------

I don't think it's the default! Without that kwarg, I get the following warning:

/g/data/hh5/public/apps/miniconda3/envs/analysis3-24.04/lib/python3.10/site-packages/xarray/coding/times.py:987: SerializationWarning: Unable to decode time axis into full numpy.datetime64 objects, continuing using cftime.datetime objects instead, reason: dates out of range
  dtype = _decode_cf_datetime_dtype(data, units, calendar, self.use_cftime)
review-notebook-app[bot] commented 1 month ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-08-30T01:22:03Z ----------------------------------------------------------------

Line #7.        darray = darray['temp'].sel(time = exp_dict[ekey]['time_bounds']) - 273.15

I think its clearer to do the time selection and temperature change on seperate lines


review-notebook-app[bot] commented 1 month ago

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-08-30T01:22:04Z ----------------------------------------------------------------

Line #25.        darray = catalog[expt].search(variable = 'u', frequency = '1mon').to_dask(xarray_open_kwargs = {'use_cftime':True})

is it clearer to combine the two functions ?

i.e. :

ds_dict = catalog[expt].search(variable = ['temp','u'], frequency = '1mon').to_dataset_dict()

ds = xr.merge(ds_dict.values())

anton-seaice commented 1 month ago

I dropped some initial thoughts, I might struggle to get time in the next week for a proper review !

julia-neme commented 1 month ago

I don't think it's the default! Without that kwarg, I get the following warning:

/g/data/hh5/public/apps/miniconda3/envs/analysis3-24.04/lib/python3.10/site-packages/xarray/coding/times.py:987: SerializationWarning: Unable to decode time axis into full numpy.datetime64 objects, continuing using cftime.datetime objects instead, reason: dates out of range
  dtype = _decode_cf_datetime_dtype(data, units, calendar, self.use_cftime)

View entire conversation on ReviewNB

julia-neme commented 1 month ago

Sorry, had a typo! Should run now.

  • © Githubissues.
  • Githubissues is a development platform for aggregating issues.