Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

gavinevans
Copy link
Contributor

@gavinevans gavinevans commented Oct 22, 2021

Addresses: #1538

Dependent upon #1572, #1582.

This is a duplicate of gavinevans#16 pointed at master. Note that this PR has been updated to include #1593.

Description
This PR builds upon work done in #1582 to add an estimate_emos_coefficients_from_table CLI that converts the pandas dataframe into an iris cube (#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)

@gavinevans gavinevans added this to the 1.0.0 milestone Oct 22, 2021
@gavinevans gavinevans self-assigned this Oct 22, 2021
@fionaRust
Copy link
Contributor

@bayliffe and @fionaRust have reviewed a previous version of this PR gavinevans#16

fionaRust
fionaRust previously approved these changes Oct 22, 2021
@codecov
Copy link

codecov bot commented Oct 22, 2021

Codecov Report

Merging #1592 (1a5db0b) into master (5a47356) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1592   +/-   ##
=======================================
  Coverage   98.05%   98.05%           
=======================================
  Files         110      110           
  Lines        9917     9944   +27     
=======================================
+ Hits         9724     9751   +27     
  Misses        193      193           
Impacted Files Coverage Δ
improver/calibration/dataframe_utilities.py 100.00% <100.00%> (ø)
improver/calibration/__init__.py 100.00% <0.00%> (ø)
improver/utilities/cube_checker.py 100.00% <0.00%> (ø)
improver/calibration/ensemble_calibration.py 100.00% <0.00%> (ø)
...semble_copula_coupling/ensemble_copula_coupling.py 99.05% <0.00%> (+<0.01%) ⬆️

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 5a47356...1a5db0b. Read the comment docs.

… 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.
…lar_ingestion_cli4

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

@fionaRust fionaRust left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with these changes. I've taken a final look through the code, reran the test and check this doesn't impact the suite,

@fionaRust fionaRust merged commit db77a0d into metoppv:master Oct 25, 2021
gavinevans added a commit to gavinevans/improver that referenced this pull request Oct 25, 2021
…lar_ingestion_cli_fp_format

* upstream/master:
  Add CLI for ingesting tabular forecasts and observations into EMOS (metoppv#1592)
  Support providing a static additional predictor when applying EMOS coefficients (metoppv#1591)
  Move dataframe to cube utilities (metoppv#1593)
  Added constant for ultraviolet_index_daytime_max. (metoppv#1590)
MoseleyS pushed a commit to MoseleyS/improver that referenced this pull request Aug 22, 2024
…etoppv#1592)

* Add CLI and acceptance tests.

* Modifications to the CLI and underlying functions to add support for 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.

* Minor style updates.

* Modifications to support the use of fastparquet at version 0.5.0.

* Revert "Modifications to support the use of fastparquet at version 0.5.0."

This reverts commit 5ccce69.

* Clean-up.

* Add test using an additional predictor.

* Add additional acceptance tests for exceptions in the CLI.

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

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

* Correct import.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants