xCDAT / xcdat

An extension of xarray for climate data analysis on structured grids.
https://xcdat.readthedocs.io/en/latest/
Apache License 2.0
101 stars 11 forks source link

Add defaults to add_missing_bounds #569

Closed acordonez closed 7 months ago

acordonez commented 7 months ago

Description

This PR adds default axes to the add_missing_bounds() function. If no axes are provided, the dataset will have X, Y, and T bounds added.

Checklist

If applicable:

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (c83f46e) 100.00% compared to head (400cebf) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #569 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 15 15 Lines 1605 1605 ========================================= Hits 1605 1605 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

acordonez commented 7 months ago

@tomvothecoder Thanks for your quick review. It look like I don't have write access to merge this.

tomvothecoder commented 7 months ago

No problem! I'll merge this PR for you.

I added you as an xCDAT org member with write access. You'll be able to work with the main repo directly instead of using your fork, which should be more convenient next time.

tomvothecoder commented 7 months ago

Also FYI that I'll need to do another xCDAT release to have this fix publicly available. I'll look into it within the next two weeks.

If I recall correctly, PCMDI Metrics developers sometimes use the local xCDAT build from the main branch to get access to the latest fixes/features. It's not recommended, but can be a temporary workaround.

lee1043 commented 7 months ago

@acordonez thank you for working on this.

If I recall correctly, PCMDI Metrics developers sometimes use the local xCDAT build from the main branch to get access to the latest fixes/features. It's not recommended, but can be a temporary workaround.

@tomvothecoder that is true, we can work with the latest version internally but PMP's new version release will be on hold until we get the latest xCDAT with this fix being incorporated. 2 weeks seems fine with us.

However, I am not sure how obs4MIPs folks are using xCDAT -- they might rely on official releases. As this fix also affects obs4MIPs, I think merging it ASAP would be appreciated. @gleckler1 Did I get this correct?

gleckler1 commented 7 months ago

obs4MIPs is in high gear, but still prototyping - previously we've modified our codes to deal with past changes to add_missing_bounds but if things are stable now so will be the obs4MIPs processing codes. I typically get xcdat from conda forge creating an env with it and cmor. @manaster may want to chime in here..

manaster commented 7 months ago

@gleckler1 this is what I do in regards to xCDAT (create and environment with xCDAT from conda forge, so I think that's the official release?). This proposed fix would seems be great for those of us working on obs4MIPs (at least in terms of our prototype code)! We were thinking we would have to make some changes to our prototype codes based on changes made to 'add_missing_bounds()' between xCDAT version 0.3.3 and xCDAT version 0.6.0 (the most recent version Peter and I have been using). The fix proposed here would seem to negate the need for those code changes as far as I can tell.

tomvothecoder commented 7 months ago

@lee1043 @gleckler1 @manaster Got it. I'll draft up a changelog for v0.6.1 and see what other small changes can go in it before releasing within the next week or two.

Tagging @xCDAT/core-developers to make everyone aware.