metoppv / improver

IMPROVER is a library of algorithms for meteorological post-processing.
http://improver.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
101 stars 84 forks source link

MOBT-639: Temporal interpolation of period diagnostics #1982

Closed bayliffe closed 4 months ago

bayliffe commented 5 months ago

Adds options for temporally interpolating period diagnostics (maxima, minima, accumulations). Ensures sensible metadata are returned.

Modifies the outputs returned. These were previously only the times to which data were interpolated, but following this change the output includes these interpolated times and the later of the two inputs cubes. This allows this later input, which for a period represents the preceding period, to be modified as well to reduce the period to that expected in the interpolated outputs. A result of this change is that in a long sequence of lead-times passed in, the first lead-time in the sequence is not made available in any output from the temporal interpolation.

Testing:

Further details: https://github.com/metoppv/mo-blue-team/issues/639 Acceptance test data: https://github.com/metoppv/improver_test_data/pull/43

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 97.46835% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 98.39%. Comparing base (209507c) to head (8569487).

Files Patch % Lines
improver/utilities/temporal_interpolation.py 97.46% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1982 +/- ## ========================================== - Coverage 98.40% 98.39% -0.01% ========================================== Files 124 124 Lines 12125 12196 +71 ========================================== + Hits 11931 12000 +69 - Misses 194 196 +2 ```

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

bayliffe commented 4 months ago

Note I have resolved conflicts this morning after merging into master the fix to the various black-formatting artefacts. Some of those fixes were in content removed in this PR, so git did not know how this PR would merge into master.

bayliffe commented 4 months ago

@benowen-bom as you've still got to look at the unit tests it is worth being aware that a great deal of the change is simply rewriting the existing tests from IrisTest style to pytest style. I've done this as a rule when I come to significantly edit unit tests as pytests is our preferred option these days.

There is a commit point in the history at the point at which I had finished rewriting the existing unit tests in the pytest style without adding any new tests. I'm not sure if you will find this helpful, but it might make clearer what's actually new and what's just a rewrite of what was there already.

bayliffe commented 4 months ago

Good spot on the extra check. I've got time interpolation blindness at this point. I've made all of your suggested changes; could you maybe have asked for something daft that I could have disputed? I look like a right pushover :-)

benowen-bom commented 4 months ago

Good spot on the extra check. I've got time interpolation blindness at this point. I've made all of your suggested changes; could you maybe have asked for something daft that I could have disputed? I look like a right pushover :-)

I'll be sure to add something daft next time! All looks good to me now, so happy to approve :-)