Update custom_transform_and_model notebook
#1216
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
🚀 Deployed on https://deploy-preview-1216--etna-docs.netlify.app |
Codecov Report
📣 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 @@ Coverage Diff @@
## master #1216 +/- ##
=======================================
Coverage 57.50% 57.50%
=======================================
Files 186 186
Lines 10614 10614
=======================================
Hits 6104 6104
Misses 4510 4510 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
View / edit / reply to this conversation on ReviewNB alex-hse-repository commented on 2023-04-11T06:42:57Z May be |
|
View / edit / reply to this conversation on ReviewNB alex-hse-repository commented on 2023-04-11T06:42:58Z May be "However, we will implement prediction decomposition as it is common component for all the models" brsnw250 commented on 2023-04-11T07:16:33Z I think the current variant suits better as we focus on showing how decomposition could be implemented if it is needed. |
|
View / edit / reply to this conversation on ReviewNB alex-hse-repository commented on 2023-04-11T06:42:59Z Line #64. df = df.loc[:, ~df.columns.duplicated()].copy() Why do you need this line? brsnw250 commented on 2023-04-11T07:18:44Z To remove duplicate columns if they are presented. It seems like some columns from previous transformations are left in ts. alex-hse-repository commented on 2023-04-11T08:38:26Z In code of the library we don't have such logic, this problem should be solved out of the model code |
|
View / edit / reply to this conversation on ReviewNB alex-hse-repository commented on 2023-04-11T06:43:00Z Line #65. timestamp_column = df["timestamp"] Columns are dropped not inplace, so it is not neccesary to save them brsnw250 commented on 2023-04-11T08:32:08Z Removed |
|
View / edit / reply to this conversation on ReviewNB alex-hse-repository commented on 2023-04-11T06:43:01Z Line #74. if return_components: May be it is better to create a separate method for components as we de in out code brsnw250 commented on 2023-04-11T07:23:55Z I think the current implementation is more straightforward and easy to follow. There is no need to jump between different methods to precisely understand how forecast works. alex-hse-repository commented on 2023-04-11T08:39:31Z I think it makes |
|
View / edit / reply to this conversation on ReviewNB alex-hse-repository commented on 2023-04-11T06:43:02Z I think we can leave only the first sentence, the other part is slightly redundant |
|
View / edit / reply to this conversation on ReviewNB alex-hse-repository commented on 2023-04-11T06:43:03Z We can use brsnw250 commented on 2023-04-11T08:31:26Z Added |
|
View / edit / reply to this conversation on ReviewNB alex-hse-repository commented on 2023-04-11T06:43:04Z Line #6. LagTransform(in_column="target", lags=list(range(21, 100, 7)), out_column="lag"), May be better to use less number of lag for better visualisation brsnw250 commented on 2023-04-11T08:31:46Z Removed some lags |
|
View / edit / reply to this conversation on ReviewNB alex-hse-repository commented on 2023-04-11T06:43:06Z We also need to show how to implement forecast decomposition for this case |
|
I think the current variant suits better as we focus on showing how decomposition could be implemented if it is needed. View entire conversation on ReviewNB |
|
To remove duplicate columns if they are presented. It seems like some columns from previous transformations are left in ts. View entire conversation on ReviewNB |
|
I think the current implementation is more straightforward and easy to follow. There is no need to jump between different methods to precisely understand how forecast works. View entire conversation on ReviewNB |
|
Added View entire conversation on ReviewNB |
|
Removed some lags View entire conversation on ReviewNB |
|
Removed View entire conversation on ReviewNB |
|
In code of the library we don't have such logic, this problem should be solved out of the model code View entire conversation on ReviewNB |
|
I think it makes View entire conversation on ReviewNB |
Before submitting (must do checklist)
Proposed Changes
Resolve first point from #1189
Closing issues