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

509 pjg clim departures #523

Closed
wants to merge 13 commits into from
Closed

509 pjg clim departures #523

wants to merge 13 commits into from

Conversation

gleckler1
Copy link
Contributor

@doutriaux1 @durack1 All intended changes are to mean_climate_metrics_calculations.py
... not sure what jupyter stuff is coming from... it can be removed before merging if that make sense

@gleckler1
Copy link
Contributor Author

@doutriaux1 @durack1 I don't understand what checks have failed here. Can one of you check and merge this so that I can close several issues and build on this?

@doutriaux1
Copy link
Contributor

doutriaux1 commented Jan 4, 2018 via email

@gleckler1
Copy link
Contributor Author

@doutriaux1 @durack1 .... more than a week... we need to be in a mode where simple merges such as this can turn around in less than a day... why are we not there?

@doutriaux1
Copy link
Contributor

@gleckler1 why did you add the JsoonClass ipynb in that PR? Why did portrait notebook changed?
Test suite fails, but that might be due to master that needs to be remerged into this.
took more than a week 'cause I was sick last week and I have one dedicated day per week. Not sure why @durack1 did not review.

@doutriaux1
Copy link
Contributor

@gleckler1 master passes test suite, so in theory we shoudln't even start reviewing until test passes. Please merge master into your branch.
Also no branch should be merged w/o a test for the new feature(s) being added...

@doutriaux1
Copy link
Contributor

@gleckler1 do we want this one?

@gleckler1
Copy link
Contributor Author

@doutriaux1 funny you should ask, I was just looking at this yesterday. The answer for the two file changes is yes. I don't know what the notebook is - I will take a look at it.

@doutriaux1
Copy link
Contributor

@gleckler1 I merged master in this one as well and fixed conflicts.

@lee1043
Copy link
Contributor

lee1043 commented Oct 5, 2021

@gleckler1 in effort of housekeeping, could you please check what's the status of this branch? Is this PR still valid?

@lee1043
Copy link
Contributor

lee1043 commented Aug 22, 2022

@gleckler1 could you remind me if this PR is still valid?

@lee1043
Copy link
Contributor

lee1043 commented Oct 11, 2022

@gleckler1 I guess this PR is inactive and should be okay to close it without merging, is that correct?

@lee1043 lee1043 marked this pull request as draft February 16, 2023 21:11
@lee1043
Copy link
Contributor

lee1043 commented Feb 16, 2023

@gleckler1 I changed this PR to draft status. It looks like key addition in this PR is to add "rms_devzm_xy","rms_devam_xy" statistics for the mean climate metrics. I will consider adding them to the xcdat version mean climate later.

@lee1043
Copy link
Contributor

lee1043 commented Feb 16, 2023

I am closing this PR and will reopen a new one once it is ready for #897.

@lee1043 lee1043 closed this Feb 16, 2023
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

Successfully merging this pull request may close these issues.

Add "rms_devzm_xy","rms_devam_xy" statistics for the mean climate metrics
3 participants