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
Implemented max_lag #43
Conversation
BSchilperoort
commented
Jul 12, 2022
•
edited
edited
- implement max_lag kwarg
- allow max_lag that exceeds 365 days, automatically skip anchor years
- write tests for new functionality
Kudos, SonarCloud Quality Gate passed! |
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.
Nice work! It is nice that by this implementation one can avoid train/test information leakage through skipping years. In the future we would have usecases that allows the overlapping of years (it is described here #44 ). But we can accommodate that easily by providing another key word argument for to enable the option. Well done 😄 !
@@ -7,6 +7,9 @@ | |||
from s2spy.time import AdventCalendar | |||
|
|||
|
|||
# pylint: disable=protected-access |
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.
This is really nice!
self._n_intervals = max_lag + self._n_target | ||
self._skip_years = np.ceil(self._n_intervals / | ||
periods_per_year).astype(int) - 1 |
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.
Nice design! In this way we avoid train/test information leakage elegantly.