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

The first_day_below and first_day_above indices should be made into generics #1175

Closed
Zeitsperre opened this issue Aug 31, 2022 · 0 comments · Fixed by #1186
Closed

The first_day_below and first_day_above indices should be made into generics #1175

Zeitsperre opened this issue Aug 31, 2022 · 0 comments · Fixed by #1186
Assignees
Labels
enhancement New feature or request standards / conventions Suggestions on ways forward
Milestone

Comments

@Zeitsperre
Copy link
Collaborator

Zeitsperre commented Aug 31, 2022

I think it would be a good idea to move the two first_day indices under _temperature.py to generic.py so that we can construct better indicators out of them.

Despite their docstrings being appropriate for general temperature data, they currently use tasmin in their call signature (since our regional focus largely concerns winter phenomena). For the wrapped indicators of these, it means that a dataset with tasmin among its data_vars will have that variable used by default. This can be misleading when using the indicator and indice forms.

If we were to move these over, I would elect to deprecate first_day_above and first_day_below and propose the following replacement, under generic.py:

  • first_day_above: def first_day_below(da: xarray.DataArray, thresh: str = None, before_date: DayOfYearStr = "07-01", window: int = 1, freq: str = "YS",) -> xarray.DataArray
  • first_day_below: def first_day_below(da: xarray.DataArray, thresh: str = None, after_date: DayOfYearStr = "07-01", window: int = 1, freq: str = "YS",) -> xarray.DataArray

under _temperature.py:

  • first_day_{tn | tg | tx}_above: def first_day_above({tasmin | tas | tasmax}: xarray.DataArray, thresh: str = None, before_date: DayOfYearStr = "07-01", window: int = 1, freq: str = "YS",) -> xarray.DataArray
  • first_day_{tn | tg | tx}_below: def first_day_below({tasmin | tas | tasmax}: xarray.DataArray, thresh: str = None, after_date: DayOfYearStr = "07-01", window: int = 1, freq: str = "YS",) -> xarray.DataArray

This would mean making some deprecation warnings for the existing indices/indicators, with breaking changes coming later.

@Zeitsperre Zeitsperre added enhancement New feature or request standards / conventions Suggestions on ways forward labels Aug 31, 2022
@Zeitsperre Zeitsperre added this to the v0.39 milestone Aug 31, 2022
@Zeitsperre Zeitsperre self-assigned this Aug 31, 2022
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.

1 participant