-
Notifications
You must be signed in to change notification settings - Fork 908
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
[DOC] Improve the explanation on the window parameter (offset) of WindowTransformer #1666
[DOC] Improve the explanation on the window parameter (offset) of WindowTransformer #1666
Conversation
expected by the window parameter of WindowTransformer Add remark to window_transformer file
considering different formats for the parameter window
@madtoinou FYI |
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #1666 +/- ##
==========================================
- Coverage 94.14% 94.05% -0.09%
==========================================
Files 125 125
Lines 11385 11371 -14
==========================================
- Hits 10718 10695 -23
- Misses 667 676 +9
... and 8 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
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 🚀 , the documentation is much clearer with this phrasing!
Minor comments about the snippet checking the ValueError
raising so that it looks similar to the approach used in the other tests
darts/tests/dataprocessing/transformers/test_window_transformations.py
Outdated
Show resolved
Hide resolved
darts/tests/dataprocessing/transformers/test_window_transformations.py
Outdated
Show resolved
Hide resolved
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, thank you for addressing the comments so quickly!
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.
Thank you @JQGoh, looks good! 🚀 I'll merge once tests have passed.
…dowTransformer (unit8co#1666) * Improve doc string to clarify the data type of offset expected by the window parameter of WindowTransformer Add remark to window_transformer file * Add test case to demonstrate the transform behaviors, considering different formats for the parameter window * Review: simply use self.assertRaises * Fix failed test --------- Co-authored-by: madtoinou <32447896+madtoinou@users.noreply.github.com> Co-authored-by: Dennis Bader <dennis.bader@gmx.ch>
#1662.
Summary
window
parameter for an offset ispandas.Timedelta
data type instead ofpandas.DateOffset
.Other Information
WindowTransformer
to demonstrate the mentioned behaviors.