Skip to content

FancierLinearRegression#2315

Open
williambdean wants to merge 3 commits intomainfrom
fancy-upgrade
Open

FancierLinearRegression#2315
williambdean wants to merge 3 commits intomainfrom
fancy-upgrade

Conversation

@williambdean
Copy link
Contributor

@williambdean williambdean commented Feb 21, 2026

Description

Migration ... but also open to removal...

Related Issue

Checklist

@cursor
Copy link

cursor bot commented Feb 21, 2026

PR Summary

Low Risk
Small migration and test update; main risk is downstream code relying on the previous fit-result variable names/import path.

Overview
FancyLinearRegression is updated to construct and document against pymc_marketing.mmm.multidimensional.MMM instead of the legacy mmm.MMM.

The associated test is adjusted for the new model outputs: the target series is explicitly named, and assertions are updated to the new InferenceData variable set (notably switching from intercept to intercept_contribution and from total_contribution to total_media_contribution_original_scale, and dropping several legacy *_original_scale contribution variables).

Written by Cursor Bugbot for commit b9262bb. This will update automatically on new commits. Configure here.

@codecov
Copy link

codecov bot commented Feb 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.05%. Comparing base (e5f50ea) to head (b9262bb).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2315   +/-   ##
=======================================
  Coverage   93.05%   93.05%           
=======================================
  Files          78       78           
  Lines       12230    12230           
=======================================
  Hits        11381    11381           
  Misses        849      849           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@williambdean williambdean requested review from ricardoV94 and removed request for ericmjl February 21, 2026 15:50
@ricardoV94
Copy link
Contributor

ricardoV94 commented Feb 21, 2026

What's the argument for keeping/removing?

If nobody uses it, sure

@ricardoV94
Copy link
Contributor

Trying to fix CI in #2316

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.

I think we should promote these baselines more. So I would not remove it.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

from pymc_marketing.mmm.components.adstock import NoAdstock
from pymc_marketing.mmm.components.saturation import NoSaturation
from pymc_marketing.mmm.mmm import MMM
from pymc_marketing.mmm.multidimensional import MMM
Copy link

Choose a reason for hiding this comment

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

Return type incompatible with publicly exported MMM class

High Severity

FancyLinearRegression now returns an instance of pymc_marketing.mmm.multidimensional.MMM, but the public API in pymc_marketing/mmm/__init__.py still exports MMM from pymc_marketing.mmm.mmm — a completely unrelated class with no shared inheritance. Users who from pymc_marketing.mmm import MMM, FancyLinearRegression will find that isinstance(FancyLinearRegression(...), MMM) returns False. The test masks this by importing MMM directly from multidimensional rather than using the public API.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think is ok, as we will work on the migration soon. Wdyt @williambdean ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, that's fine.

@juanitorduz juanitorduz added this to the 1.0 milestone Feb 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants