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

Enh/ecad wind indices #230

Merged
merged 11 commits into from
Oct 24, 2022
Merged

Enh/ecad wind indices #230

merged 11 commits into from
Oct 24, 2022

Conversation

bzah
Copy link
Member

@bzah bzah commented Oct 18, 2022

Pull Request is related to #204

  • Unit tests cover the changes.
  • These changes were tested on real data.
  • The relevant documentation has been added or updated.
  • A short description of the changes has been added to doc/source/references/release_notes.rst.

Describe the changes you made

Add ECAD wind indices.

Notes

  1. The definition used for DDnorth in icclim is different from the one from ECAD ATBD v11[0].
    It seems the ATBD wrongly assumes that DD can be below 0 degree in DDnorth but that should not be possible.
    According to both chapter 3 of the ATBD [0] and ERA5 documentation [1] DD is bounded by [0, 360] degree.

  2. DDsouth definition is unclear in the ATBD.
    In icclim we assume DDsouth is (135° < DD <= 225°) and not (225° < DD <= 315°)

[0] https://knmi-ecad-assets-prd.s3.amazonaws.com/documents/atbd.pdf
[1] https://confluence.ecmwf.int/pages/viewpage.action?pageId=133262398

@github-actions
Copy link

github-actions bot commented Oct 18, 2022

Coverage

Report
FileStmtsMissCoverMissing
icclim
   icclim_exceptions.py7186%20
   icclim_logger.py852966%24, 43, 65, 70–95, 105, 111–142, 150–155
   main.py2702093%119, 128, 403, 488, 514–515, 517–518, 520, 522–523, 525–526, 545–552, 689, 696, 700
   utils.py29197%20
icclim/generic_indices
   generic_indicators.py4698482%57, 60, 77, 81, 85, 98, 104, 178, 217, 224, 355, 384, 666, 697, 703, 711, 730, 808, 828–834, 838, 868, 870, 935–936, 969–996, 1023–1044, 1060, 1069–1070, 1076–1106, 1112–1116, 1140
   standard_variable.py46198%19
   threshold.py3982195%198, 225, 229, 277, 292–295, 310, 317, 393, 529, 598, 692, 895, 917, 923, 997, 1000, 1098–1100
icclim/models
   cf_calendar.py35197%20
   climate_variable.py961090%102, 146, 157, 191, 197, 235, 239, 267–268, 305
   frequency.py1761194%167–170, 314–316, 323, 401, 427, 432
   index_group.py34197%33
   operator.py29197%14
   standard_index.py28293%59, 63
icclim/pre_processing
   input_parsing.py1783381%41, 105, 110, 116, 125, 138, 167, 183, 197, 217, 225, 227, 236, 245, 263, 277–285, 294–309, 331
   rechunk.py86298%38, 132
TOTAL241721891% 

Test results

Tests Skipped Failures Errors Time
185 0 💤 0 ❌ 0 🔥 2m 2s ⏱️

@bzah bzah added this to the 6.0 milestone Oct 19, 2022
@bzah bzah removed this from the 6.0 milestone Oct 20, 2022
It's a waste of ressources to build at import time the thresholds of ECAD indices.
- It's clearer now how each parameter of thresholds is built.
- Made use of PercentileThreshold inheritance to improve type hinting
- Improve import statements by truly importing the function/classes in the script
  and using builtin methods to access their name and module.
  This doesn't work with types as they don't have builtin __name__ in them though.
@bzah bzah merged commit de3c189 into master Oct 24, 2022
@bzah bzah deleted the enh/ecad_wind_indices branch October 24, 2022 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant