bug #112: window size identification fixed for trend change detection #114
Conversation
for trend change detection
|
Hey @sayanchk, @shahsmit14, @snazzyfox I opened this PR to solve the bug regarding the data exploration module for time series with Weekly frequency. I'm kindly asking you to review it and approve it. Thank you very much. |
|
Hi @papaemman, I think the example in the other PR (also adding here as reference) is comprehensive. My point is the assumption of monthly seasonality for weekly data could be little stringent. That being said, based on the example you shared, I think it's fare to go with your changes and see how it works out for your and other use cases. A better solution would be to check the _detect_window_size method within DataExploration and optimize the method to detect the window sizes automatically for weekly data. I'll create an issue and tag you to it for visibility. Thanks for your contribution to Luminaire! |
|
Here's the link to the related issue i just created: #115 |
|
@shahsmit14 I added you as a reviewer to this issue as well. Looks like I don't have permission to merge it. Please approve and merge if it looks okay on your end. Thanks! |
|
@shahsmit14 @snazzyfox Following up for updates. Can anyone take a look? |
|
Sorry for the delayed response. Merging this. |
|
I will do a release this with this MR since it has the code to release for v0.4.1 in setup.py |
|
Pending one more merge before releasing #118 |
|
@papaemman v0.4.1 is now available to use. :) |
The current approach for trend detection in the Data exploration module (
/luminaire/exploration/data_exploration.py) was enabled only for daily ('D') and hourly ('H) time series.I added a fix to trigger the computation of window sizes for weekly ('W') frequency, using a value of 4.
Also, I added a fix to support all the other frequencies.
This is related to issue Weekly data exploration failure #112
This PR is to replace the PR Related to issue #112: Exploration failure for weekly data #113