Skip to content

Conversation

rbeucher
Copy link
Contributor

@rbeucher rbeucher commented Jun 6, 2024

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.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

review-notebook-app bot commented Aug 30, 2024

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)

Copy link

review-notebook-app bot commented Aug 30, 2024

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


Copy link

review-notebook-app bot commented Aug 30, 2024

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
Copy link
Collaborator

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

Copy link
Collaborator

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

Copy link
Collaborator

@navidcy navidcy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to run this recipe but it fails at cell 12. I pushed the notebook with the error message.

@julia-neme
Copy link
Collaborator

Sorry, had a typo! Should run now.

Copy link
Collaborator

@navidcy navidcy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm; thanks @julia-neme

@navidcy navidcy merged commit 84b3320 into main Sep 1, 2024
3 checks passed
@navidcy navidcy deleted the INTAKE_Equatorial_thermal_and_zonal_velocity_structure branch September 1, 2024 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants