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.

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

Fix _create_synth_dataset when MMM has no controls #1513

Merged
merged 3 commits into from
Feb 19, 2025

Conversation

PabloRoque
Copy link
Contributor

@PabloRoque PabloRoque commented Feb 19, 2025

Description

MMM._create_synth_dataset does not work in MMMs without controls.

Note the underscore difference.

Before:

        if controls is not None:
            _controls: list[str] = controls
        else:
            controls = []

After:

        if controls is not None:
            _controls: list[str] = controls
        else:
            _controls = [] 

Added a test and fixtures to test the method in MMMs without controls in the future.

Related Issue

Checklist


📚 Documentation preview 📚: https://pymc-marketing--1513.org.readthedocs.build/en/1513/

@PabloRoque PabloRoque added bug Something isn't working MMM labels Feb 19, 2025
@PabloRoque PabloRoque self-assigned this Feb 19, 2025
@github-actions github-actions bot added the tests label Feb 19, 2025
@wd60622
Copy link
Contributor

wd60622 commented Feb 19, 2025

There is an issue for this. Can you link

Copy link

codecov bot commented Feb 19, 2025

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.74%. Comparing base (26e80c0) to head (0ff9fcc).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pymc_marketing/mmm/mmm.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1513   +/-   ##
=======================================
  Coverage   92.74%   92.74%           
=======================================
  Files          52       52           
  Lines        6120     6123    +3     
=======================================
+ Hits         5676     5679    +3     
  Misses        444      444           

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

@PabloRoque
Copy link
Contributor Author

There is an issue for this. Can you link

I was not aware of the opened issue. Linked now.

@PabloRoque PabloRoque requested a review from wd60622 February 19, 2025 17:22
@github-actions github-actions bot added good first issue Good for newcomers . Doesn't require extensive knowledge of the repo and package optimizer priority: high labels Feb 19, 2025
@wd60622 wd60622 added this to the 0.12.0 milestone Feb 19, 2025
Copy link
Contributor

@wd60622 wd60622 left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks!

@wd60622 wd60622 merged commit 44ed2ea into pymc-labs:main Feb 19, 2025
18 of 20 checks passed
@PabloRoque PabloRoque deleted the fix-create-synth-dataset branch February 19, 2025 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers . Doesn't require extensive knowledge of the repo and package MMM optimizer priority: high tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

budget allocation with no control vars
2 participants