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

Oi indicators #129

Merged
merged 6 commits into from
Apr 12, 2024
Merged

Oi indicators #129

merged 6 commits into from
Apr 12, 2024

Conversation

richardarsenault
Copy link
Contributor

@richardarsenault richardarsenault commented Mar 30, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes #xyz
  • (If applicable) Documentation has been added / updated (for bug fixes / features).
  • (If applicable) Tests have been added.
  • CHANGES.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?

Added support for optimal interpolation of hydrological indicators.

Does this PR introduce a breaking change?

No.

Other information:

Added some flexibility for some parameters that were hard coded and are not optional inputs.

tests/test_optimal_interpolation.py Outdated Show resolved Hide resolved
tests/test_optimal_interpolation.py Outdated Show resolved Hide resolved
xhydro/optimal_interpolation/ECF_climate_correction.py Outdated Show resolved Hide resolved
xhydro/optimal_interpolation/ECF_climate_correction.py Outdated Show resolved Hide resolved
xhydro/optimal_interpolation/ECF_climate_correction.py Outdated Show resolved Hide resolved
xhydro/optimal_interpolation/optimal_interpolation_fun.py Outdated Show resolved Hide resolved
xhydro/optimal_interpolation/optimal_interpolation_fun.py Outdated Show resolved Hide resolved
xhydro/optimal_interpolation/optimal_interpolation_fun.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@RondeauG RondeauG left a comment

Choose a reason for hiding this comment

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

Ça me semble bon ! Il va juste falloir que tu mettes à jour CHANGES.rst avant de merger.

@github-actions github-actions bot added the approved Approved for additional tests label Apr 11, 2024
@RondeauG
Copy link
Collaborator

On dirait que le notebook pour cette PR a été mis dans #128 par accident ?

@richardarsenault
Copy link
Contributor Author

@RondeauG Hmmm oui on dirait qu'il y a une copie du notebook pour l'OI dans ravenpy mais il est déjà mergé dans main... je ne suis pas trop certain comment ça s'est passé mais je peux le retirer de ravenpy models!

@richardarsenault richardarsenault merged commit a62eccf into main Apr 12, 2024
23 checks passed
@richardarsenault richardarsenault deleted the oi-indicators branch April 12, 2024 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants