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.

Sign up for GitHub

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

Jump to bottom

Make cell_methods-derived bounds optional in getvar #294

Merged
merged 1 commit into from
Jun 17, 2022
Merged

Make cell_methods-derived bounds optional in getvar #294

merged 1 commit into from
Jun 17, 2022

Conversation

angus-g
Copy link
Collaborator

@angus-g angus-g commented Jun 17, 2022

This is following the suggestion from #284 (comment), to be able to return a Dataset if we need things like time_bounds, otherwise default to a DataArray and don't attach additional attributes. I don't know if return_dataset is a particularly enlightening name, happy to take suggestions there...

Closes #284.

@aidanheerdegen
Copy link
Collaborator

I can only think of worse names than return_dataset. Like return_type='DataArray' which is kinda awful. return_bounds is more specific, but makes it harder to add other related variables (like curvilinear grid coords) if that was considered useful in the future, and doesn't indicate the type of the object returned from the function changes.

@angus-g
Make cell_methods-derived bounds optional in getvar
1674459 
This is particularly useful for ice data where we can get the time
bounds for averaging intervals. However, because we were attaching the
bounds as an xarray Dataset, we could't serialise a variable straight
back to disk, which was catching people out. Instead, we can make the
bounds optional, only if you want them. In this case, return a full
xarray Dataset, rather than just a DataArray.

Closes COSIMA#284.
@aidanheerdegen
Copy link
Collaborator

Looks good. I am happy to merge or let you do it.

@angus-g angus-g merged commit a59f276 into COSIMA:master Jun 17, 2022
@angus-g angus-g deleted the bounds-dataset branch June 17, 2022 06:20
@angus-g angus-g mentioned this pull request Sep 13, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aidanheerdegen aidanheerdegen aidanheerdegen left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add time_bounds breaks serialisation
2 participants
@angus-g @aidanheerdegen