-
Notifications
You must be signed in to change notification settings - Fork 59
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 season and days above #1708
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I wish there was an easier way to have a transitional set of functions, but there isn't really. Your approach makes sense.
) | ||
|
||
snw_days_above = SnowWithIndexing( | ||
title="Days with snow (amount)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Days with snow greater than amount"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved!
Co-authored-by: Éric Dupuis <[email protected]>
@Zeitsperre it seems the doctests do not see the testing data ? Do you know what is happening ? |
@aulemahal I think the doctests are flakier than we imagined. It looks like there are data races / unsafe asynchronous read events that are causing problems. Re-running the failed tests might help for here, but we should address this ASAP. Will open an issue. |
The |
Pull Request Checklist:
number
) and pull request (:pull:number
) has been addedWhat kind of change does this PR introduce?
sn[dw]_season_length
tosn[dw]_days_above
, which better reflects what they actually do.sn[dw]_season_length
as an actual season length : duraction between a start and an end, both defined as the first day of a period of minimum length continuously above/under a given threshold.snw
, like we said we'd do in 0.47 (youpsi).Does this PR introduce a breaking change?
Yes. An indicator name now points to another computation.
Other information: