Add CLI for ingesting tabular forecasts and observations into EMOS #16

gavinevans · 2021-10-19T16:14:10Z

Dependent upon metoppv#1572, metoppv#1582.

Description
This PR builds upon work done in metoppv#1582 to add an estimate_emos_coefficients_from_table CLI that converts the pandas dataframe into an iris cube (metoppv#1582) and passes that iris cube to the existing EMOS functionality.

Note that this PR does contain an update to acceptance.py to support providing a directory as a known good output in the acceptance tests. This is required for compatibility with a partitioned parquet file.

Please note that the decision about whether to use fastparquet 0.5.0 or 0.7.1 is subject to change due to issues with the Conda environments (https://github.com/MetOffice/improver_suite/pull/1026). The first three commits in this PR assumed the use of 0.7.1. This commit reverts the version of fastparquet to 0.5.0.

Further information is available in this comment.

Testing:

Ran tests and they passed OK
Added new tests for the new feature(s)

fionaRust · 2021-10-21T13:59:46Z

I'm reviewing this

codecov-commenter · 2021-10-22T07:34:06Z

Codecov Report

Merging #16 (9c98027) into improver1538_tabular_ingestion_functions (8b134fa) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@                             Coverage Diff                              @@
##           improver1538_tabular_ingestion_functions      #16      +/-   ##
============================================================================
+ Coverage                                     98.04%   98.08%   +0.03%     
============================================================================
  Files                                           109      110       +1     
  Lines                                          9872    10069     +197     
============================================================================
+ Hits                                           9679     9876     +197     
  Misses                                          193      193

Impacted Files	Coverage Δ
improver/ensemble_copula_coupling/constants.py	`100.00% <ø> (ø)`
improver/calibration/__init__.py	`100.00% <100.00%> (ø)`
improver/calibration/dataframe_utilities.py	`100.00% <100.00%> (ø)`
improver/calibration/ensemble_calibration.py	`100.00% <100.00%> (ø)`
improver/wxcode/weather_symbols.py	`96.88% <100.00%> (+0.08%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f63415a...9c98027. Read the comment docs.

… of forecast and observation tables. (metoppv#1572)

bayliffe

Goodness you've written a lot of code for all of this calibration work. Two cli exceptions need tests, otherwise this looks fine to me. I've not run the tests as I am waiting on very slow conda dependency solving.

bayliffe · 2021-10-22T11:54:32Z

improver/cli/estimate_emos_coefficients_from_table.py

+            f"The requested filepath {forecast} does not contain the "
+            f"requested contents: {filters}"
+        )
+        raise IOError(msg)


This is not tested.

Test added,

bayliffe · 2021-10-22T11:54:36Z

improver/cli/estimate_emos_coefficients_from_table.py

+            f"The requested filepath {truth} does not contain the "
+            f"requested contents: {filters}"
+        )
+        raise IOError(msg)


This is not tested.

Test added.

…s cube (metoppv#1582) * Modifications to functions required to support converting tabular data into iris cubes. * Modifications to the columns expected within the truth table. * Modify environments in preparation for changes required for ingestion of forecast and observation tables. * Edits to expect the period column to be a timedelta64 dtype. * Corrections to __init__.py * Modifications to use assertCubeEqual to ensure the full cubes are compared. * Modifications following review comments. * Minor updates to docstrings. * Add missing unit tests. * Correction to csv files. * Sort lists. * Minor docstring amendment. * NOT WORKING: Working commit to try to avoid using the truth altitude, latitude and longitude at all and replace with those from the forecast. * Extend docstrings. * Extended documentation updates. * Refinement and addition of tests for column name checking. * Modifications to tidy up dataframe preparation. * Minor extended documentation edits. * Further minor docstring edit. * Correct test class naming. * Fix isort.

fionaRust · 2021-10-21T16:55:56Z

improver/cli/estimate_emos_coefficients_from_table.py

+            iterations may require increasing, as there will be more
+            coefficients to solve.
+        percentiles (List[float]):
+            The set of percentiles to be used for estimating EMOS coefficients.


Suggested change

The set of percentiles to be used for estimating EMOS coefficients.

The set of percentiles to be used for estimating EMOS coefficients.

These should be a set of equally spaced quantiles.

I've updated this docstring.

fionaRust · 2021-10-21T16:56:46Z

improver/calibration/__init__.py

@@ -340,6 +365,8 @@ def _prepare_dataframes(
            DataFrame expected to contain the following columns: ob_value,
            time, wmo_id, diagnostic, latitude, longitude and altitude.
            Any other columns are ignored.
+        percentiles:
+            The set of percentiles to be used for estimating EMOS coefficients.


Suggested change

The set of percentiles to be used for estimating EMOS coefficients.

The set of percentiles to be used for estimating EMOS coefficients.

These should be a set of equally spaced quantiles.

I've updated this docstring.

fionaRust · 2021-10-22T13:35:09Z

improver/cli/estimate_emos_coefficients_from_table.py

+        forecast_period,
+        training_length,
+        percentiles=percentiles,
+    )


I think as we discussed last week, some leadtime-cycle combinations won't be possible. This will result in no cubes being returned from this function. In this case we should just return and not produce output

You're correct that this is handling within the plugins by None being returned, although this is a bit trickier within the CLIs, as the CLIs are expecting to save a netCDF file, if a --output is provided. I've made a minor edit to skip the saving if the result is None.

…passing in a list of percentiles, improvements to the unit tests to make them more reflective of the intended input files, checks to ensure the percentiles can be considered to be quantiles and other improvements.

…5.0." This reverts commit 5ccce69.

… dependencies between the calibration and ECC code.

…rover1538_tabular_ingestion_cli4 * improver1538_move_tabular_ingestion_functions: Move dataframe to cube utilities to a separate file to avoid circular dependencies between the calibration and ECC code.

* Move dataframe to cube utilities to a separate file to avoid circular dependencies between the calibration and ECC code. * Minor docstring updates.

…lar_ingestion_cli4 * upstream/master: Move dataframe to cube utilities (metoppv#1593) Added constant for ultraviolet_index_daytime_max. (metoppv#1590)

gavinevans self-assigned this Oct 19, 2021

gavinevans mentioned this pull request Oct 19, 2021

Add CLI for ingesting tabular forecasts and observations into EMOS metoppv/improver#1584

Closed

2 tasks

gavinevans removed their assignment Oct 21, 2021

gavinevans force-pushed the improver1538_tabular_ingestion_functions branch from 8b134fa to ed31c06 Compare October 21, 2021 17:35

gavinevans force-pushed the improver1538_tabular_ingestion_cli4 branch from 5288575 to 23cb8c1 Compare October 22, 2021 07:47

Modify environments in preparation for changes required for ingestion…

052a038

… of forecast and observation tables. (metoppv#1572)

bayliffe suggested changes Oct 22, 2021

View reviewed changes

gavinevans mentioned this pull request Oct 22, 2021

Support ingestion of tabular site forecasts and observations into EMOS metoppv/improver#1538

Closed

3 tasks

fionaRust suggested changes Oct 22, 2021

View reviewed changes

bayliffe approved these changes Oct 22, 2021

View reviewed changes

gavinevans mentioned this pull request Oct 22, 2021

Add CLI for ingesting tabular forecasts and observations into EMOS metoppv/improver#1592

Merged

2 tasks

fionaRust approved these changes Oct 22, 2021

View reviewed changes

gavinevans added 9 commits October 22, 2021 16:33

Add CLI and acceptance tests.

d419d34

Minor style updates.

c5bf236

Modifications to support the use of fastparquet at version 0.5.0.

103e3f4

Revert "Modifications to support the use of fastparquet at version 0.…

d60c98a

…5.0." This reverts commit 5ccce69.

Clean-up.

74bbb41

Add test using an additional predictor.

b683f30

Add additional acceptance tests for exceptions in the CLI.

784a662

Update docstrings and add support for returning None from a CLI.

a390238

gavinevans force-pushed the improver1538_tabular_ingestion_cli4 branch from 5ddf334 to a390238 Compare October 22, 2021 15:34

gavinevans and others added 5 commits October 22, 2021 18:42

Move dataframe to cube utilities to a separate file to avoid circular…

992072e

… dependencies between the calibration and ECC code.

Merge branch 'improver1538_move_tabular_ingestion_functions' into imp…

6d0f279

…rover1538_tabular_ingestion_cli4 * improver1538_move_tabular_ingestion_functions: Move dataframe to cube utilities to a separate file to avoid circular dependencies between the calibration and ECC code.

Correct import.

9c98027

Added constant for ultraviolet_index_daytime_max. (metoppv#1590)

555a085

Move dataframe to cube utilities (metoppv#1593)

5a47356

* Move dataframe to cube utilities to a separate file to avoid circular dependencies between the calibration and ECC code. * Minor docstring updates.

Merge remote-tracking branch 'upstream/master' into improver1538_tabu…

1a5db0b

…lar_ingestion_cli4 * upstream/master: Move dataframe to cube utilities (metoppv#1593) Added constant for ultraviolet_index_daytime_max. (metoppv#1590)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CLI for ingesting tabular forecasts and observations into EMOS #16

Add CLI for ingesting tabular forecasts and observations into EMOS #16

gavinevans commented Oct 19, 2021 •

edited

Loading

fionaRust commented Oct 21, 2021

codecov-commenter commented Oct 22, 2021 •

edited

Loading

bayliffe left a comment

bayliffe Oct 22, 2021

gavinevans Oct 22, 2021

bayliffe Oct 22, 2021

gavinevans Oct 22, 2021

fionaRust Oct 21, 2021

gavinevans Oct 22, 2021

fionaRust Oct 21, 2021

gavinevans Oct 22, 2021

fionaRust Oct 22, 2021

gavinevans Oct 22, 2021

	The set of percentiles to be used for estimating EMOS coefficients.
	The set of percentiles to be used for estimating EMOS coefficients.
	These should be a set of equally spaced quantiles.

Add CLI for ingesting tabular forecasts and observations into EMOS #16

Are you sure you want to change the base?

Add CLI for ingesting tabular forecasts and observations into EMOS #16

Conversation

gavinevans commented Oct 19, 2021 • edited Loading

fionaRust commented Oct 21, 2021

codecov-commenter commented Oct 22, 2021 • edited Loading

Codecov Report

bayliffe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gavinevans commented Oct 19, 2021 •

edited

Loading

codecov-commenter commented Oct 22, 2021 •

edited

Loading