Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clean up timeseries access #38

Open
matt-long opened this issue May 11, 2020 · 1 comment
Open

clean up timeseries access #38

matt-long opened this issue May 11, 2020 · 1 comment

Comments

@matt-long
Copy link
Contributor

matt-long commented May 11, 2020

ann_avg_utils has unnecessary input parameter:
experiment_longnames

Also, the user pulls things like units out of the module, then puts them back into the model via this API. I would prefer that the routine returns a dictionary of xarray datasets with the units conversion already applied. I am applying the unit conversion post facto:

for var in variables:
    for exp in cesm2_exp:        
        ds = dsets[var][exp]
        
        var_units = ds[var].attrs['units']
        
        var_data_new_units = (
            ds[var].data * pint_units[var_units]
        ).to(final_units[var]).magnitude
        
        ds[var].values = var_data_new_units
        new_units = final_units[var]
        if new_units in pretty_units:
            new_units = pretty_units[new_units]
        ds[var].attrs['units'] = new_units

This routine doesn't seem to actually be computing timeseries, but rather constructing cache file names. I would prefer that the computation happens, if necessary, here.

Must we do any concatenation inside this function? It seems like mission creep.

@matt-long
Copy link
Contributor Author

matt-long commented May 11, 2020

Presuming it's not replaced by a broader revision, I think get_ann_means_and_units should be renamed to something like get_timeseries and possibly accept a freq argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant