-
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
Save off media transformations #882
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #882 +/- ##
==========================================
+ Coverage 94.52% 94.58% +0.05%
==========================================
Files 33 33
Lines 3345 3378 +33
==========================================
+ Hits 3162 3195 +33
Misses 183 183 ☔ 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.
Thanks @wd60622
@wd60622 we can merge this one after we you solve the merge conflicts :) |
pre-commit.ci autofix |
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.
Any thoughts on this change? @juanitorduz
If the adstock and saturation keys are not present than we could inject a default. However, I am not sure that is good idea
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.
Well, I think we need this to recover the model after saving it, right? What could be the drawbacks of injecting the default?
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.
Injecting a default would likely not recreate the same model at all
Any idea if the model |
We actually do not have any explicit convention. I usually do it when there is a big refactor, but we can change that. |
* to_dict via lookup_name * parse to and from dict for attrs * improve the codecov * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * change test with change in default behavior * increase the MMM model version --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* to_dict via lookup_name * parse to and from dict for attrs * improve the codecov * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * change test with change in default behavior * increase the MMM model version --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* to_dict via lookup_name * parse to and from dict for attrs * improve the codecov * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * change test with change in default behavior * increase the MMM model version --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Description
Add
to_dict
method for media transformation and leverage in theMMM
in order to save off configurations completelyRelated Issue
Checklist
Modules affected
Type of change
📚 Documentation preview 📚: https://pymc-marketing--882.org.readthedocs.build/en/882/