-
Notifications
You must be signed in to change notification settings - Fork 192
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
Time slice cross validation MMM Notebook #971
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #971 +/- ##
==========================================
+ Coverage 95.59% 95.62% +0.02%
==========================================
Files 35 36 +1
Lines 3499 3517 +18
==========================================
+ Hits 3345 3363 +18
Misses 154 154 ☔ View full report in Codecov by Sentry. |
e2fdaa0
to
d1fbfd8
Compare
65dd7c2
to
e869f5f
Compare
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.
Overall looks really good. Some questions. Looking more into the notebook now
View / edit / reply to this conversation on ReviewNB wd60622 commented on 2024-08-29T03:35:59Z "It t can be" -> "It can be"
Also, are people suppose to use the juanitorduz commented on 2024-08-29T07:27:24Z I think usually people woill look into the average (crps) |
View / edit / reply to this conversation on ReviewNB wd60622 commented on 2024-08-29T03:35:59Z Line #6. apply_sklearn_transformer_across_dim( This is the only way to do this currently? We don't have it wrapped up? :( |
View / edit / reply to this conversation on ReviewNB wd60622 commented on 2024-08-29T03:36:00Z "Event hough" -> "Even though" juanitorduz commented on 2024-08-29T07:28:18Z Thanks! |
I think usually people woill look into the average (crps) View entire conversation on ReviewNB |
Thanks! View entire conversation on ReviewNB |
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.
Looks great. Thanks for the adjustments
Thank you @wd60622 for the review and feedback 🙏 |
* nb init * add param stability * crps init * add crps to package * make mypy happy * clean * relax req * update path * add crps test * first iteration * improvements * improve tests * improve tests * improve references * feedback 1 * add examples
* nb init * add param stability * crps init * add crps to package * make mypy happy * clean * relax req * update path * add crps test * first iteration * improvements * improve tests * improve tests * improve references * feedback 1 * add examples
* nb init * add param stability * crps init * add crps to package * make mypy happy * clean * relax req * update path * add crps test * first iteration * improvements * improve tests * improve tests * improve references * feedback 1 * add examples
Closes #956
📚 Documentation preview 📚: https://pymc-marketing--971.org.readthedocs.build/en/971/