Cmorize gpcc fix

[mwjury]

This fix improves cmorization of GPCC based on:

• correct treatment of time points and bounds

Checklist

It is the responsibility of the author to make sure the PR is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.

• 🛠 PR has a descriptive title for the changelog
• 🛠 Code follows the style guide
• 🛠 Circle/CI tests pass
• 🛠 Codacy code quality checks pass

New or updated data reformatting script:

• 🛠 Dataset is added to the table in the documentation
• 🛠 Documentation contains instructions to obtain the data
• 🛠 Tests for the CMORized data are available
• 🧪 Numbers/units of the data look physically meaningful
• 🛠 Data set is added to the OBS data pool @remi-kazeroni

[Peter9192]

#1980 is causing the failed tests.

[bouweandela]

To fix the failing tests on CircleCI, you'll need to merge the latest master branch into this branch.

[valeriupredoi]

cheers @mwjury this looks good - it'd be nice to have a test here too, to test the new functionality 🍺 EDIT - nevermind the test, there is no test module for it anyway 👍

[valeriupredoi]

cheers @mwjury - I will merge once we merge the fix to the iris documentation from #2012 🍺

[remi-kazeroni]

I will also try this cmorizer and put the data at DKRZ and CEDA before we merge this

[valeriupredoi]

great! cheers muchly @remi-kazeroni 🍺

[remi-kazeroni]

Nothing to do here. The data were already available in the OBS directories as this PR is a fix to a cmorizer.

[valeriupredoi]

excellent, cheers @remi-kazeroni -> @mwjury now that #2011 has been fixed it would be good if you merged master here and in all your other PRs that are ready for merging please, so we merge with clean tests 👍

[bouweandela]

The data were already available in the OBS directories as this PR is a fix to a cmorizer.

@remi-kazeroni If this fix is needed, I would expect that the data currently in the OBS directories is wrong and needs to be updated after this has been merged?

[bouweandela]

Just applying some new labels to make the status of all open pull requests more clear. Note that the tests on CircleCI and the documentation build must be successful (green) before the label 'approved by technical reviewer' can be applied.

[remi-kazeroni]

@remi-kazeroni If this fix is needed, I would expect that the data currently in the OBS directories is wrong and needs to be updated after this has been merged?

Good point @bouweandela. Would you suggest that I only look at the cmorizer PRs once they are merged and apply the merged cmorizer script to produce cmorized data that I store in OBS directories? One issue is that sometimes the data used to develop the cmorizer is no longer available online and replaced by a newer version (with different names). Recent examples being #1977 #1851 or #1991. So if the cmorizer has not been checked against available data shortly before the merge, it may not be useful in pratice due to data version issues.

[bouweandela]

apply the merged cmorizer script to produce cmorized data that I store in OBS directories?

Yes, I think that would be the best approach. That way we at least have a copy of the script used to CMORize the data in the OBS directories, that will make the process more reproducible and debugging that data easier.

Would you suggest that I only look at the cmorizer PRs once they are merged

It would be better if you could take an active interest in these pull requests, because there are currently many open and the progress on getting them merged is extremely slow. But it's up to you to decide how much time you can spend on this of course.

So if the cmorizer has not been checked against available data shortly before the merge, it may not be useful in practice due to data version issues.

Maybe we could make it the rule that CMORizer pull requests are always merged by you, so you can run a final check to make sure that everything is in order just before pressing the merge button?

[remi-kazeroni]

Yes, I think that would be the best approach. That way we at least have a copy of the script used to CMORize the data in the OBS directories, that will make the process more reproducible and debugging that data easier.

Agreed. It would be nice to merge a script that is consistent with the data available at the time of the merge.

It would be better if you could take an active interest in these pull requests, because there are currently many open and the progress on getting them merged is extremely slow. But it's up to you to decide how much time you can spend on this of course.

Sure, I'll do my best to contribute to these PRs.

Maybe we could make it the rule that CMORizer pull requests are always merged by you, so you can run a final check to make sure that everything is in order just before pressing the merge button?

That is what I had in mind. I'm fine with that. Thanks for your clarification @bouweandela

[bouweandela]

Thanks @remi-kazeroni! I have created a pull request to clarify this in the documentation here: #2017.

[valeriupredoi]

@remi-kazeroni I changed the label to "sci review in progress" - that's you, mate, as soon as you happy with the data versioning bits this can be merged 👍

[remi-kazeroni]

Everything fine. Good to merge 👍

[valeriupredoi]

@mwjury could you please do this next time: when you have a branch that fixes something off another unmerged branch (like #1995 fixed bits in here) please merge the two branches together and open a single pull request that contains all needed changes; this avoids conflicts and the reviewers depending on merging branches into other branches to be able to test; this would be welcomed, if changes in the branch that fixes stuff in the other branch are minor ie not much code, as is the case with #1995 fixing bits in #1982 (here) 👍 Otherwise, if two branches need to be kept, please provide a note which one of them needs to be merged first, that'll help avoiding conflicts as well 🍺