-
Notifications
You must be signed in to change notification settings - Fork 541
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
Anomaly Detection: Add Daily Season with Hourly Interval to HoltWinter #546
Conversation
rdsharma26 Hi Can you help review this PR? It would be great if HoltWinter support more seasonality and interval. Also having a custom seriesPeriodicity would allow others to construct their own seasonality without always having to make changes to source code |
@zeotuan Thanks for the PR! The change looks good. However, would it be possible to add a unit test that shows how a custom value for |
@rdsharma26 Thanks. Tests Added for Custom seriesPeriodicity and Hourly Interval/Daily Seasonality |
src/test/scala/com/amazon/deequ/anomalydetection/seasonal/HoltWintersTest.scala
Outdated
Show resolved
Hide resolved
@@ -207,6 +207,73 @@ class HoltWintersTest extends AnyWordSpec with Matchers { | |||
anomalies should have size 3 | |||
} | |||
} | |||
|
|||
"work on hourly data with daily seasonality" in { |
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.
Can you add a test for custom seriesPeriodicity
as well? The two tests are testing the original constructor and not the new constructor.
I have addressed the comments. |
Thanks for addressing the comments. LGTM. |
#546) * Add Daily Season with Hourly Interval, Add custom periodicity constructor * Add Custom seriesPeriodicity, Hourly interval with Daily seasonality tests * Add Test using custom seriesPeriodicity
#546) * Add Daily Season with Hourly Interval, Add custom periodicity constructor * Add Custom seriesPeriodicity, Hourly interval with Daily seasonality tests * Add Test using custom seriesPeriodicity
#546) * Add Daily Season with Hourly Interval, Add custom periodicity constructor * Add Custom seriesPeriodicity, Hourly interval with Daily seasonality tests * Add Test using custom seriesPeriodicity
#546) * Add Daily Season with Hourly Interval, Add custom periodicity constructor * Add Custom seriesPeriodicity, Hourly interval with Daily seasonality tests * Add Test using custom seriesPeriodicity
#546) * Add Daily Season with Hourly Interval, Add custom periodicity constructor * Add Custom seriesPeriodicity, Hourly interval with Daily seasonality tests * Add Test using custom seriesPeriodicity
#546) * Add Daily Season with Hourly Interval, Add custom periodicity constructor * Add Custom seriesPeriodicity, Hourly interval with Daily seasonality tests * Add Test using custom seriesPeriodicity
#546) * Add Daily Season with Hourly Interval, Add custom periodicity constructor * Add Custom seriesPeriodicity, Hourly interval with Daily seasonality tests * Add Test using custom seriesPeriodicity
Add Daily Season with Hourly Interval to HoltWinter: #296
Add constructor that directly take seriesPeriodicity
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.