Skip to content

chore: remove dead catch_warnings blocks from add_groups migration#2652

Open
williambdean wants to merge 2 commits into
v1.0.0from
chore/cleanup-dead-code
Open

chore: remove dead catch_warnings blocks from add_groups migration#2652
williambdean wants to merge 2 commits into
v1.0.0from
chore/cleanup-dead-code

Conversation

@williambdean

@williambdean williambdean commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

The 'fit_data not defined in InferenceData scheme' warning was emitted by arviz's add_groups, which was replaced by idata["/fit_data"] = fit_data in the PyMC 6 migration (#2616). DataTree assignment never raises this warning, making the catch_warnings / filterwarnings blocks dead code.

Removed in: mixed_logit.py, mnl_logit.py, nested_logit.py, maxdiff.py.

Flagged by @daimon-pymclabs in #2616 review.


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

- The 'fit_data not defined in InferenceData scheme' warning was
  emitted by arviz add_groups, which has been replaced by
  idata["/fit_data"] = fit_data. DataTree assignment never raises it.
- Removed dead with/catch_warnings/filterwarnings blocks in
  mixed_logit, mnl_logit, nested_logit, maxdiff.
@github-actions github-actions Bot added the customer choice Related to customer choice module label Jun 26, 2026
- Same 'fit_data not defined in InferenceData scheme' warning
  blocks no longer needed in conftest fixtures after add_groups migration.
@github-actions github-actions Bot added the tests label Jun 26, 2026
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.07%. Comparing base (1e2b2ed) to head (bf04485).

Additional details and impacted files
@@            Coverage Diff             @@
##           v1.0.0    #2652      +/-   ##
==========================================
- Coverage   94.07%   94.07%   -0.01%     
==========================================
  Files          97       97              
  Lines       14617    14606      -11     
==========================================
- Hits        13751    13740      -11     
  Misses        866      866              

☔ View full report in Codecov by Harness.
📢 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

Copy link
Copy Markdown
Contributor Author

Please take a look @daimon-pymclabs

@daimon-pymclabs

Copy link
Copy Markdown
Contributor

Took a look — this is good to merge. Checked the mechanical side carefully:

  • The four customer_choice fit() methods and the two conftest fixtures all suppressed the same UserWarning ("The group fit_data is not defined in the InferenceData scheme"), which no longer fires now that fit_data is a recognized group — so the blocks are genuinely dead.
  • The import warnings removals are correct: dropped from mixed_logit.py, mnl_logit.py, nested_logit.py and both conftests where it's now unused, and retained in maxdiff.py (still used by warnings.warn(...) at ~L601). No dangling references left in any changed file.
  • Full test matrix + customer_choice notebooks are green, so the suppressed path is exercised.

One optional belt-and-suspenders: a quick run of the customer_choice fit tests under -W error::UserWarning would prove the warning is gone rather than merely silent, but I don't think it's necessary to block on. LGTM 👍

@williambdean williambdean requested a review from juanitorduz June 27, 2026 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

customer choice Related to customer choice module tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants