IMPRO-1779 get local day #1375
IMPRO-1779 get local day #1375
Conversation
- Adds method and test to build the output_cube with appropriate meta-data
- Adds support for multiple utc_times
- And tidies up a bit
- Removes support for multiple output UTC times (no use-case) - Adds tests to ensure input cubes are compatible
- Input datetime is now called local_time as this better describes what it is for - time_units definition moved to __init__ as not dependent on any inputs
…cal-day * upstream/master: Demotion of datatypes in "collapsed" wrapper (metoppv#1371) WeatherSymbols handles masked data. (metoppv#1366) IMPRO 1870: Sleet in global wxcode tree (metoppv#1365)
Codecov Report
@@ Coverage Diff @@
## master #1375 +/- ##
=======================================
Coverage 96.29% 96.29%
=======================================
Files 89 89
Lines 7476 7559 +83
=======================================
+ Hits 7199 7279 +80
- Misses 277 280 +3
Continue to review full report at Codecov.
|
improver/cli/map_to_timezones.py
Outdated
| Must have the same spatial coords as input_cube. | ||
| Use generate-timezone-mask-ancillary to create this. | ||
| local_time (str): | ||
| The "local" time of the output cube as %Y%m%dT%H%MZ. This will form a |
There was a problem hiding this comment.
Local time, but not in a local time zone (ends with Z indicating UTC)?
There was a problem hiding this comment.
I hadn't really intended to get reviews on this just yet as I'm still working on it. This "local_time" thing is still giving me a headache as it just doesn't fit very nicely. Getting rid of the Z is a good idea, but it still populates a "utc" coord that doesn't mean UTC.
| coord = iris.coords.DimCoord( | ||
| np.array([offset], dtype=np.int32), | ||
| long_name="UTC_offset", | ||
| units="hours", |
There was a problem hiding this comment.
There are a few political entities that have non-unit-hours UTC offsets - do we ever need to worry about those?
There was a problem hiding this comment.
As mentioned already in this bit of code, we aren't producing data at a finer temporal resolution than one hour, so sub-hour time-zone offsets need rounding to a whole-hour or we won't find a match for them.
…ut_cube to be run last.
- Tests input as either cube or list - Tests input with and without bounds on the time coord
…al_timezone" - Updates code and unit-tests - Updates doc-strings - Updates add_coordinate method to be more explicit in which time coords require an update of forecast_period coord - Updates acceptance test data checksums
…9-get-local-day * remotes/upstream/master: Least significant digit (metoppv#1369) Unsafe night mask (metoppv#1370)
…ant-digit change.
…d_coordinate method for this simple requirement.
- Improves an error message - Tweaks doc-strings - Simplifies unit-tests slightly
There was a problem hiding this comment.
This is a nice speedy bit of code. I've not gone through the tests yet. Please can you address the main functional issue and update the unit tests / test data. Then I will go through everything again.
I've also discovered silly bugs in the ancillary creation tool for the old 0-360 grids that need to be fixed (well done me), though thankfully they don't affect us with current data, so I'll write that up and do it later.
- Corrects calculation of local-time using the offset (reverses sign)
- Recreates acceptance test inputs and KGO - Because the UK input times were asymmetric, I needed to recreate them. Because I used pytest parameterize (and I got the data from the Alpha suite), I therefore had to recreate the global too.
- Improves doc-strings and comments - Refactors dtype enforcement from temporal.py and cube_combiner.py into check_datatypes.py
…nput cube. - Modifies the unit tests so that the dimensions are not all len(3) as this can mask broadcasting errors.
- Modifies plugin to copy any cell_methods on the input_cube onto the output_cube - Improves doc-strings and comments
| def check_input_cube_dims(self, input_cube, timezone_cube): | ||
| """Ensures input cube has at least three dimensions: time, y, x. Promotes time | ||
| to be the inner-most dimension (dim=-1). Does the same for the timezone_cube | ||
| UTC_offset dimension. |
There was a problem hiding this comment.
I think this function requires one more test, which is to ensure that the spatial coordinates match. As we perform the calculation here by just multiplying arrays rather than cubes, it is currently possible to apply a timezone cube made on a 0 to 360 degree grid to a -180 to 180 degree grid. A check here will save us from this mistake.
There was a problem hiding this comment.
I'll move the call to spatial_coords_match from process to here. It belongs here better - I agree.
There was a problem hiding this comment.
I see that you have moved it. Unfortunately that call doesn't perhaps do what you think it does. It simply returns True or False, which is not in and of itself much of a test. The code continues regardless of the result. You need to raise an exception if the result is not what you want, and this needs a unit test.
There was a problem hiding this comment.
So it does! Now it will raise an error (added a test for this too).
…9-get-local-day * remotes/upstream/master: Shower probability (metoppv#1380) Updated spp_string in threshold.py to differentiate between less_than… (metoppv#1372) Scalar threshold percentile (metoppv#1374) Rename hybridtotcld (metoppv#1376) IMPRO-1870: Removes documentation IDs from wxcode decision trees (metoppv#1373)
* 'master' of github.com:metoppv/improver: Wxcode reordering (metoppv#1378) IMPRO-1779 get local day (metoppv#1375)
* Starts to add a TimezoneExtraction plugin * Sets up output_cube - Adds method and test to build the output_cube with appropriate meta-data * Sets up output_cube - Adds support for multiple utc_times * Removes xy-coord function (unnecessary) - And tidies up a bit * Completes plugin and unit-tests. - Removes support for multiple output UTC times (no use-case) - Adds tests to ensure input cubes are compatible * Adds check for spatial coords * Improves doc-string * isort * Improves documentation - Input datetime is now called local_time as this better describes what it is for - time_units definition moved to __init__ as not dependent on any inputs * Adds dtype checking * generate_timezone_mask now outputs masks as int8, not int32 * generate_timezone_mask now outputs cubes with UTC_offset coords that have units * generate_timezone_mask now outputs cubes with correctly-defined UTC_offset coords. * Adds CLI and acceptance tests * Adds unit-test for TypeError * Adds handling for time-bounds on input_cube and refactors create_output_cube to be run last. * Bug fix and update of checksums following inclusion of time-bounds. * Removes the trailing Z from input local-time argument. * Extends unit-test coverage - Tests input as either cube or list - Tests input with and without bounds on the time coord * Changes coord representation of local time from "utc" to "time_in_local_timezone" - Updates code and unit-tests - Updates doc-strings - Updates add_coordinate method to be more explicit in which time coords require an update of forecast_period coord - Updates acceptance test data checksums * Updates acceptance-test checksums following merge with least-significant-digit change. * Simplifies creation of time_in_local_timezone coord to avoid using add_coordinate method for this simple requirement. * First review - Improves an error message - Tweaks doc-strings - Simplifies unit-tests slightly * Second review - Corrects calculation of local-time using the offset (reverses sign) * Second review - Recreates acceptance test inputs and KGO - Because the UK input times were asymmetric, I needed to recreate them. Because I used pytest parameterize (and I got the data from the Alpha suite), I therefore had to recreate the global too. * Second review - Improves doc-strings and comments - Refactors dtype enforcement from temporal.py and cube_combiner.py into check_datatypes.py * Modifies code to handle an extra dimension (e.g. percentile) on the input cube. - Modifies the unit tests so that the dimensions are not all len(3) as this can mask broadcasting errors. * Second review - Modifies plugin to copy any cell_methods on the input_cube onto the output_cube - Improves doc-strings and comments * isort eyesore a puddy tat * Methods that aren't tested directly are now private methods. * Second review: Moves call to spatial_coords_match into check_input_cube_dims. * Updates checksums for cell-method change. * Corrects test for spatial coords mismatch so that it raises an error if necessary.
Adds plugin and CLI to map input data onto local timezones.
Modifies timezone generation ancillary plugin to output int8 data and to contain units on the UTC_offset coord.
Modifies add_coordinate method to be more specific about which coord names require the "forecast_period" coord to be recreated.
Testing: