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

Move dataframe to cube utilities #1593

Conversation

gavinevans
Copy link
Contributor

@gavinevans gavinevans commented Oct 22, 2021

Addresses #1538

Description
Move the dataframe to cube utilities to a separate file to avoid circular imports between the calibration and ECC code, which causes unexpected import errors.

This was due to a conflict between the lines below, which wasn't picked up in any of the testing of #1582.

# In calibration/__init__.py
from improver.ensemble_copula_coupling.ensemble_copula_coupling import RebadgePercentilesAsRealizations
# In improver.ensemble_copula_coupling.ensemble_copula_coupling
from improver.calibration.utilities import convert_cube_data_to_2d

To test that the issue has been resolved, try:

from improver.ensemble_copula_coupling.ensemble_copula_coupling import RebadgePercentilesAsRealizations

Testing:

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

… dependencies between the calibration and ECC code.
@gavinevans gavinevans added this to the 1.0.0 milestone Oct 22, 2021
@gavinevans gavinevans self-assigned this Oct 22, 2021
@codecov
Copy link

codecov bot commented Oct 22, 2021

Codecov Report

Merging #1593 (cd1d126) into master (f6f90ce) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1593   +/-   ##
=======================================
  Coverage   98.05%   98.05%           
=======================================
  Files         109      110    +1     
  Lines        9914     9917    +3     
=======================================
+ Hits         9721     9724    +3     
  Misses        193      193           
Impacted Files Coverage Δ
improver/calibration/__init__.py 100.00% <100.00%> (ø)
improver/calibration/dataframe_utilities.py 100.00% <100.00%> (ø)

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 f6f90ce...cd1d126. Read the comment docs.

@gavinevans gavinevans marked this pull request as ready for review October 25, 2021 07:24
@fionaRust fionaRust requested review from bayliffe and Kat-90 October 25, 2021 08:00
Copy link
Contributor

@bayliffe bayliffe left a comment

Choose a reason for hiding this comment

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

Doc-string stuff.

improver/calibration/__init__.py Show resolved Hide resolved
improver/calibration/dataframe_utilities.py Outdated Show resolved Hide resolved
@gavinevans gavinevans assigned Kat-90 and unassigned gavinevans Oct 25, 2021
Copy link
Contributor

@Kat-90 Kat-90 left a comment

Choose a reason for hiding this comment

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

I have run the tests, run the import command and reviewed the code. Happy with these changes.

@Kat-90 Kat-90 assigned gavinevans and unassigned Kat-90 Oct 25, 2021
@bayliffe bayliffe merged commit 5a47356 into metoppv:master Oct 25, 2021
gavinevans added a commit to gavinevans/improver that referenced this pull request Oct 25, 2021
…lar_ingestion_cli4

* upstream/master:
  Move dataframe to cube utilities (metoppv#1593)
  Added constant for ultraviolet_index_daytime_max. (metoppv#1590)
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
* Move dataframe to cube utilities to a separate file to avoid circular dependencies between the calibration and ECC code.

* Minor docstring updates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants