MOBT-719: Frequency to count for time period averages #2006
Conversation
…onversion using time bound information. Adds a CLI and unit tests.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2006 +/- ##
==========================================
+ Coverage 98.39% 98.43% +0.03%
==========================================
Files 124 128 +4
Lines 12212 12489 +277
==========================================
+ Hits 12016 12293 +277
Misses 196 196 ☔ View full report in Codecov by Sentry. |
…for the integrate time bounds function.
improver/utilities/temporal.py
Outdated
| time bounds over which it is defined to return a count or accumulation. | ||
| The frequency or rate must be defined with time bounds, e.g. an average | ||
| frequency across the period. This function will handle a cube with a | ||
| non-scalar time coordinate, multiplying each time in the coordiante by the |
There was a problem hiding this comment.
| non-scalar time coordinate, multiplying each time in the coordiante by the | |
| non-scalar time coordinate, multiplying each time in the coordinate by the |
There was a problem hiding this comment.
When will you get onboard with my respelling of coordianté? Note the accent in this version which I think gives it a lovely sound. But fine...
| time bounds over which it is defined to return a count or accumulation. | ||
| The frequency or rate must be defined with time bounds, e.g. an average | ||
| frequency across the period. This function will handle a cube with a | ||
| non-scalar time coordinate, multiplying each time in the coordiante by the |
There was a problem hiding this comment.
| non-scalar time coordinate, multiplying each time in the coordiante by the | |
| non-scalar time coordinate, multiplying each time in the coordinate by the |
There was a problem hiding this comment.
Not yet apparently.
| raise ValueError( | ||
| "time coordinate must have bounds to apply this time-bounds integration" | ||
| ) | ||
|
|
There was a problem hiding this comment.
At the moment, I don't think anything is stopping users from providing a rate/frequency which is (for example) 'per hour', which would lead to unexpected results.
Could an additional check be added along the lines of:
| if str(cube.units).find('s-') == -1: | |
| raise ValueError('message') |
A corresponding unit test would also be required to demonstrate this functionality.
There was a problem hiding this comment.
The code will modify the units of the returned cube to ensure that any input unit can be handled. The data will be integrated in seconds, but the unit will include a factor that accounts for the different input units, that why the unit is modified using the * Unit("s") approach. I had failed to demonstrate this in the units tests, so I've added a relevant test.
…ect doc-string typos.
* Adds a small utility for rate to accumulation or frequency to count conversion using time bound information. Adds a CLI and unit tests. * Add style fixes. Add a unit test for the exception. * Add acceptance tests and fixup the CLI. * Update checksums, style fix. * Reorder test statement. * Added unit test coverage of preservation and removal of cell methods for the integrate time bounds function. * Add a test to demonstrate how irregular input units are handled. Correct doc-string typos.
This change adds a function and CLI to multiply average rates / frequencies within periods by the length of the period over which they are defined to yield an accumulation / count over the same period. This is to be used in the first instance for converting lightning flash frequencies into counts.
Acceptance test data: metoppv/improver_test_data#49
Testing: