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

Snow and radiation conversion, indicator translation fixes #1271

Merged
merged 30 commits into from Feb 3, 2023

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Jan 13, 2023

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes #xyz
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • HISTORY.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

  • Adds two new indices and indicators (snow_depth_from amount and snow_amount_from_depth) for converting between snow amount and snow depth (with the aid of snow density).
  • Adds two new indices and indicators (longwave_radiation_upwards_from_net_downwards and shortwave_radiation_upwards_from_net_downwards) for determining rlus from rls and rlds, as well as rsus from rss and rsds, respectively.
  • Adds the [power] unit definition for radiation-based variables.
  • Fixed a bug that was affecting the validity checking of some indicator translations in test_xclim_translations.
    • Added missing translations for kbdi, df, ffdi and fixed broken translations for cffwis and jetstream_metric_woolings.
  • Some minor adjustments in docstrings

Does this PR introduce a breaking change?

No.

Other information:

See: Ouranosinc/miranda#106

@Zeitsperre Zeitsperre added the enhancement New feature or request label Jan 13, 2023
@Zeitsperre Zeitsperre added this to the v0.41 milestone Jan 13, 2023
@Zeitsperre Zeitsperre self-assigned this Jan 13, 2023
@github-actions github-actions bot added the indicators Climate indices and indicators label Jan 13, 2023
xclim/indices/_conversion.py Outdated Show resolved Hide resolved
xclim/indices/_conversion.py Outdated Show resolved Hide resolved
xclim/indices/_conversion.py Outdated Show resolved Hide resolved
xclim/indices/_conversion.py Outdated Show resolved Hide resolved
xclim/indices/_conversion.py Outdated Show resolved Hide resolved
@Zeitsperre Zeitsperre changed the title Snow conversion Snow and radiation conversion Feb 1, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added the docs Improvements to documenation label Feb 2, 2023
Copy link
Collaborator

@aulemahal aulemahal left a comment

Choose a reason for hiding this comment

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

While looking up the CF standard for rlus and family, I realized we were not using the same vocabulary... I think we should go for CF:
"surface downwelling shortwave flux" instead of "surface downwards solar radiation", and so on.

"Downwards" sound like a direction, while "downwelling" also has a meaning of source.

For snow conversion, I don't think these are "approximations". I would rename:

snow_amount_approximation -> snow_amount
snow_depth approximation -> snow_depth
snow_depth -> snow_depth_mean

(sry)

xclim/data/fr.json Outdated Show resolved Hide resolved
xclim/data/fr.json Outdated Show resolved Hide resolved
xclim/indicators/atmos/_conversion.py Outdated Show resolved Hide resolved
xclim/indicators/atmos/_conversion.py Outdated Show resolved Hide resolved
xclim/indices/_conversion.py Outdated Show resolved Hide resolved
xclim/indices/_conversion.py Outdated Show resolved Hide resolved
Zeitsperre and others added 2 commits February 2, 2023 16:30
Co-authored-by: Pascal Bourgault <bourgault.pascal@ouranos.ca>
xclim/data/fr.json Outdated Show resolved Hide resolved
@Zeitsperre Zeitsperre changed the title Snow and radiation conversion Snow and radiation conversion, indicator translation fixes Feb 3, 2023
HISTORY.rst Outdated Show resolved Hide resolved
@github-actions github-actions bot added the approved Approved for additional tests label Feb 3, 2023
@Zeitsperre Zeitsperre merged commit 0728404 into master Feb 3, 2023
@Zeitsperre Zeitsperre deleted the snow_conversion branch February 3, 2023 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests docs Improvements to documenation enhancement New feature or request indicators Climate indices and indicators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants