Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow non-Prior parameters in Prior and media transformations #1292

Merged
merged 9 commits into from
Dec 18, 2024
Merged

Allow non-Prior parameters in Prior and media transformations #1292

merged 9 commits into from
Dec 18, 2024
+193 −17

Conversation

wd60622
Copy link
Contributor

@wd60622 wd60622 commented Dec 17, 2024 •
edited
Loading

Description

import numpy as np
import pymc as pm

from pymc_marketing.mmm import GeometricAdstock

adstock = GeometricAdstock(l_max=12, priors={"alpha": 1.0})

X = np.random.randn(100)

with pm.Model() as model:
    adstocked_X = adstock.apply(X)

Some plotting will not be supported if there are no distribution parameters required

Related Issue

  • Closes #
  • Related to #

Checklist

Modules affected

  • MMM
  • CLV

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

@wd60622 wd60622 requested a review from juanitorduz December 17, 2024 17:37
@github-actions github-actions bot added the MMM label Dec 17, 2024
@wd60622 wd60622 changed the title All non-Prior in media transformations Allow non-Prior in media transformations Dec 17, 2024
@codecov Codecov
Copy link

codecov bot commented Dec 17, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 89.47368% with 4 lines in your changes missing coverage. Please review.

Project coverage is 94.96%. Comparing base (4066b3a) to head (ff1835e).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
pymc_marketing/mmm/components/base.py 86.95% 3 Missing ⚠️
pymc_marketing/prior.py 90.90% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1292      +/-   ##
==========================================
- Coverage   95.02%   94.96%   -0.07%     
==========================================
  Files          42       42              
  Lines        4462     4488      +26     
==========================================
+ Hits         4240     4262      +22     
- Misses        222      226       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions github-actions bot added the tests label Dec 17, 2024
juanitorduz
juanitorduz requested changes Dec 17, 2024
View reviewed changes
Copy link
Collaborator

@juanitorduz juanitorduz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice! Two small requests :) WDYT?

pymc_marketing/mmm/components/base.py Outdated
@@ -117,7 +120,9 @@ class Transformation:
lookup_name: str

def __init__(
self, priors: dict[str, Prior] | None = None, prefix: str | None = None
self,
priors: Prior | float | TensorVariable | None = None,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please update the type hints in the doctrines 🙏 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

pymc_marketing/mmm/components/base.py Show resolved Hide resolved
@wd60622 wd60622 changed the title Allow non-Prior in media transformations Allow non-Prior parameters in Prior and media transformations Dec 18, 2024
juanitorduz
juanitorduz approved these changes Dec 18, 2024
View reviewed changes
Copy link
Collaborator

@juanitorduz juanitorduz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Than you @wd60622 !

@juanitorduz juanitorduz merged commit 298fe2a into main Dec 18, 2024
18 of 19 checks passed
@juanitorduz juanitorduz deleted the allow-non-prior-media-input branch December 18, 2024 19:41
This was referenced Dec 19, 2024
Open
Closed
aseyboldt pushed a commit to aseyboldt/pymc-marketing that referenced this pull request Dec 20, 2024
@wd60622 @juanitorduz @aseyboldt
Allow non-Prior parameters in Prior and media transformations (pymc-l…
03e4063 
…abs#1292)

* relax constraint while creating distributions

* allow for non-prior class inputs in media transformations

* test for serialization

* test for arbitrary object

* relax what is needed and define protocol

* add test for arbitrary

* update the docstring and type hints

* test for the serialization

---------

Co-authored-by: Juan Orduz <[email protected]>
@wd60622 wd60622 mentioned this pull request Dec 20, 2024
Closed
@wd60622 wd60622 added the enhancement New feature or request label Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juanitorduz juanitorduz juanitorduz approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request MMM Prior class tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wd60622 @juanitorduz