-
Notifications
You must be signed in to change notification settings - Fork 220
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
Add to documentation feature comparison #550
Add to documentation feature comparison #550
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #550 +/- ##
=======================================
Coverage 91.36% 91.36%
=======================================
Files 21 21
Lines 2073 2073
=======================================
Hits 1894 1894
Misses 179 179 ☔ 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 👍
Actually, Recast does have time-varying coefficients (they have said so a couple of times). Hence I will mark it as well instead of “?” |
I would also add a “is unit-tested” 😂 as Robyn does not have a single test. Red flag 🚩 |
Sounds good. As for the others, do you know the status? |
All tested except Recast which is not public, so maybe “?” |
Hey @juanitorduz are you sure about the TVP? I understand based on what they mentioned here, they have it. https://mmm-hub.slack.com/archives/C05RDDE1GUB/p1708474546559629?thread_ts=1707851321.022429&cid=C05RDDE1GUB |
Yes! I agree! This is what I meant above. They do have time-varying coefficients in some flavour. My last comment was about unit-tests :) |
Pretty sure they do. Was in a sales call with them some months ago and the guy made it very clear that they had TVP. |
Looks great! |
@wd60622 ready to merge this one ? |
Ready whenever you are! |
Done! Thanks @wd60622 ! |
@juanitorduz This PR should probably have been squashed merged, no point in having 6 dirty commits in the git history |
Oh! You are right! I'm usually very careful with this! I'm sorry 🙈. Can we ever and maybe merge again? |
Better to let it stay as is. I was just flagging it in case it wasn't a one time slip. I also do it now and then |
Ok! Thanks for the remainder anyway 💪 |
Description
The current MMM features were outdated and add some additional rows for o
Still think we should discuss if this should be included at all as it might be hard to maintain
Related Issue
Checklist
Modules affected
Type of change
📚 Documentation preview 📚: https://pymc-marketing--550.org.readthedocs.build/en/550/