Check presence of time bounds and guess them if needed

[sloosvel]

A check for the presence of time bounds has been added. If they are required but they are not present, they are guessed with the following criteria:

• hourly, 3hourly, 6hourly and daily data: the bounds are set considering the corresponding delta time.

• Monthly data: the minimum bound is set at the first day of the month at 00h. The maximum bound is set at the first day of the next month at 00h.

• Yearly data: the minimum bound is set at the first day of the year at 00h. The maximum bound is set at the first day of the next year at 00h.

• Decadal data: the minimum bound is set at the first day of the year at 00h. The maximum bound is set at the first day of the next ten years at 00h.
  Before you start, please read our contribution guidelines.

Tasks

• Create an issue to discuss what you are going to do, if you haven't done so already (and add the link at the bottom)
• This pull request has a descriptive title that can be used in a changelog
• Add unit tests
• Public functions should have a numpy-style docstring so they appear properly in the API documentation. For all other functions a one line docstring is sufficient.
• If writing a new/modified preprocessor function, please update the documentation
• Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
• Codacy code quality checks pass. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.
• Please use yamllint to check that your YAML files do not contain mistakes
• If you make backward incompatible changes to the recipe format, make a new pull request in the ESMValTool repository and add the link below

If you need help with any of the tasks above, please do not hesitate to ask by commenting in the issue or pull request.

---

Closes #802 , closes #803

[Peter9192]

Hi @sloosvel let's see if we can get this merged. I can do a round of review (already had a quick look, seems good). But before I continue with testing etc., can you

• merge master and see if there are any remaining failing tests, and
• see if you can address the codacy issues?

In general, I think your approach at guessing the bounds looks very decent. Actually, another place in the code where we guess time bounds is in the regrid time preprocessor:

ESMValCore/esmvalcore/preprocessor/_time.py

Line 721 in 694f993

cube.coord('time').guess_bounds()

I'd say your approach here looks better, and I wonder if we could/should somehow make them consistent and at the same time avoid code duplication. Any thoughts on that?

[sloosvel]

I'd say your approach here looks better, and I wonder if we could/should somehow make them consistent and at the same time avoid code duplication. Any thoughts on that?

I guess some lines could be moved as functions to either _time.py or _shared.py?

[sloosvel]

I don't think I can fix the remaining Codacy issues.

[Peter9192]

Great, now the tests pass :-)

I guess some lines could be moved as functions to either _time.py or _shared.py?

Yes, a _guess_time_bounds function in _time.py sounds like a great feature to me. I think that'll also solve the 1001-line codacy complaint, and the others are indeed not new so I'm okay with leaving those.

[Peter9192]

Hey @sloosvel,
This PR does what it says, so in principle I'm already convinced we should merge it. I just have a few remaining questions that I'm hoping you could help me with, especially regarding the tests.

[sloosvel]

Is there anything left to address here? I added new tests as requested.

[Peter9192]

I'll try to have another look today or otherwise early next week. Last time I checked I wasn't sure if you were still working on it.

[sloosvel]

No problem! I kind of put it aside because of things I had pending for the release.

[Peter9192]

Looks good! All tests pass locally. I just merged master to verify that they also pass on CI with the most recent changes.

[Peter9192]

Okay just one more thing: could you have another look at the codacy issue? I don't see the circular imports if I try to move it to the top. All greens would help to get it merged :-)

[Peter9192]

Thanks @sloosvel happy to see this merged :-)