Refactor first_day indices into generics #1186
Conversation
…iables they work upon, source DayOfYearStr from same source, add deprecation notices
Zeitsperre
added
enhancement
…for generic indice, fix warnings
There was a problem hiding this comment.
Suggestion, if you like it : To further generalize this, we could use the indicator-creation "input" arg to transform the generic function directly into an indicator. This way, you wouldn't need all the indices.
Example:
first_day_tn_below = Temp(
identifier="first_day_tn_below",
units="",
standard_name="day_of_year",
long_name="First day of year with minimum temperature below {threshold}",
description="First day of year with minimum temperature below {threshold} for at least {window} days.",
title="First day of year with minimum temperature below a threshold.",
abstract="Returns first day of period where minimum temperature is inferior to a threshold over a given number of days, limited to a starting calendar date.",
compute=indices.generic.first_day_below,
input={'data': 'tasmin'},
parameters=dict(threshold={"default": "0 degC"}, after_date={"default": "07-01"}),
)Also. We already have "tn_days_below", wouldn't it be coherent to name the new indicators like: "tn_first_day_below" ?
…|below indice to reduce redundancy
xclim/indices/generic.py
Outdated
| @@ -808,3 +813,107 @@ def degree_days(tas: xr.DataArray, threshold: str, op: str) -> xr.DataArray: | |||
| raise NotImplementedError(f"Condition not supported: '{op}'.") | |||
|
|
|||
| return to_agg_units(out, tas, op="delta_prod") | |||
|
|
|||
|
|
|||
| def first_day_above( | |||
There was a problem hiding this comment.
To go one step further into generification you could have instead a first_day_threshold_reached with an operator parameter taking either > or < as a valid value.
It would reduce the duplication of code and doc.
This would also ease the adding of new operators such as >=, <=, ==
It similar to what is done on degree_days for example.
There was a problem hiding this comment.
Absolutely! Will make changes. Thanks!
…'op' for indices, rewrite tests and indicators to support op
|
This is now ready for final review |
Co-authored-by: Abel Aoun <aoun@cerfacs.fr>
Pull Request Checklist:
first_day_belowandfirst_day_aboveindices should be made into generics #1175number) and pull request (:pull:number) has been addedAUTHORS.rstand.zenodo.jsonWhat kind of change does this PR introduce?
DayOfYearStrfrom same file sourceDoes this PR introduce a breaking change?
Yes.
first_day_threshold_reachedis a new generic indice, and users must transition to usingfirst_day_temperature_belowandfirst_day_temperature_abovefor previous behaviour. For previous indices/indicators (first_day_above|below), they remain available, but will showDeprecationWarningswhen called by user and will be removed in xclim>=0.40.0Other information:
The- no longer relevantfirst_day_XX_above|belowindices all perform local imports of genericfirst_day_aboveandfirst_day_below. This was needed to ensure that the deprecated versionsof these indices remain available without overwriting the newer generics. This will no longer be needed in xclim>=0.40.0