-
Notifications
You must be signed in to change notification settings - Fork 53
Alternative xarray approach to computing convservation time series #1080
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
Conversation
…ries This alternative approach makes sure the constents are unique (and sorted) in the xtime variable. To reduce redundancy between the new approach and the old ncrcat approach (retained for comparison and in case the xarray approach is not performant), a new helper function for determining if we want to append to the existing datasets added.
TestingI ran just the I am also running the test suite and the conservation analysis looks the same as before in tests that have completed so far there: As an example, here is a plot of salt conservation prior to this PR: |
|
@cbegeman and @darincomeau, if you could have a look, that would be great. If we're good with these fixes, I can merge this and update the analysis environment for BlueTip to include this fix. |
cbegeman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving on the basis of code inspection and @xylar's testing. Thanks for fixing this!
|
Thanks @cbegeman! |
|
Thanks @xylar for the quick fix! You're testing looks good to me. I can approve now (it'll be easier to use this in |
|
I think you can just approve. It's not too hard for me to redo thing if something goes wrong later with the updated BlueTip environment. |
darincomeau
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved based on developer testing. Thanks again Xylar!
|
Thanks @darincomeau. I'll try to put a new environment in place soon. |


This alternative approach makes sure the contents are unique (and sorted) in the
xtimevariable.To reduce redundancy between the new approach and the old
ncrcatapproach (retained for comparison and in case thexarrayapproach is not performant), a new helper function for determining if we want to append to the existing datasets added.Checklist
Testingcomment in the PR documents testing used to verify the changes