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

Refactoring maximum_consecutive_XYZ_days indices #1386

Closed
4 tasks done
coxipi opened this issue Jun 7, 2023 · 0 comments · Fixed by #1443
Closed
4 tasks done

Refactoring maximum_consecutive_XYZ_days indices #1386

coxipi opened this issue Jun 7, 2023 · 0 comments · Fixed by #1443
Assignees
Labels
enhancement New feature or request standards / conventions Suggestions on ways forward
Milestone

Comments

@coxipi
Copy link
Contributor

coxipi commented Jun 7, 2023

Generic Issue

PR #1359 adds many new spell indices. Many indices like: maximum_consecutive_dry_days and other similar cases are just special cases of a spell index (150eede). This might be something to consider as we try to refactor xclim (and perhaps have less indices/ more re-used assets).

List of indices

  • maximum_consecutive_{dry|frost|tx|wet}_days could use: {dry|cold|hot|wet}_spell_max_length

  • maximum_consecutive_frost_free_days: There is no frost_free_spell_max_length but it could be added

  • {tg|tn|tx}_{max|mean|min} , max_1day_precipitation_amount, {snw|snd}_max,

    # these are all one-liner functions e.g. tx_min 
    return tasmax.resample(time=freq).min(dim="time").assign_attrs(units=tasmax.units)

    could use generic.select_resample_op.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@coxipi coxipi changed the title Refactoring maximum_consecutive indices Refactoring maximum_consecutive_XYZ_days indices Jun 7, 2023
@coxipi coxipi changed the title Refactoring maximum_consecutive_XYZ_days indices Refactoring maximum_consecutive_XYZ_days indices Jun 7, 2023
@Zeitsperre Zeitsperre added enhancement New feature or request standards / conventions Suggestions on ways forward labels Jun 7, 2023
@Zeitsperre Zeitsperre added this to the Summer 2023 milestone Jun 13, 2023
@Zeitsperre Zeitsperre modified the milestones: Summer 2023, v1.0 Jul 25, 2023
coxipi added a commit that referenced this issue Aug 30, 2023
<!--Please ensure the PR fulfills the following requirements! -->
<!-- If this is your first PR, make sure to add your details to the
AUTHORS.rst! -->
### Pull Request Checklist:
- [x] This PR addresses an already opened issue (for bug fixes /
features)
    - This PR fixes #1386
- [x] Tests for the changes have been added (for bug fixes / features)
- [x] (If applicable) Documentation has been added / updated (for bug
fixes / features)
- [x] CHANGES.rst has been updated (with summary of main changes)
- [x] Link to issue (:issue:`number`) and pull request (:pull:`number`)
has been added

### What kind of change does this PR introduce?

* Refactoring + new indices (similar to already existing ones)

### Does this PR introduce a breaking change?
`integral` replaces `prod` and `delta_prod` options as `op` arguments in
`to_agg_units`

### Other information:
Few corrections in french translations. Also,
`generic.select_resample_op` now keeps attrs in both use cases (`op` is
a `str` or a function )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request standards / conventions Suggestions on ways forward
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants