Skip to content

fix LaTeX dot in diversification ratio formula #1788

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
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix LaTeX dot in diversification ratio formula #1788

wants to merge 4 commits into from
+3 −3

Conversation

saishreyakumar
Copy link

@saishreyakumar saishreyakumar commented Jun 19, 2025 •
edited by github-actions bot
Loading

Description

Replaced \cdot with . in the LaTeX formula for the diversification ratio in the docstring to fix the rendering issue.

Related Issue

Checklist

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

@github-actions github-actions bot added MMM docs Improvements or additions to documentation good first issue Good for newcomers . Doesn't require extensive knowledge of the repo and package labels Jun 19, 2025
@codecov Codecov
Copy link

codecov bot commented Jun 19, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.07%. Comparing base (893a7f2) to head (8245df2).
Report is 7 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1788   +/-   ##
=======================================
  Coverage   92.07%   92.07%           
=======================================
  Files          62       62           
  Lines        7398     7398           
=======================================
  Hits         6812     6812           
  Misses        586      586

☔ 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
Copy link
Contributor

Thanks for the edits @saishreyakumar

I'm see some issues with it still
https://pymc-marketing--1788.org.readthedocs.build/en/1788/api/generated/pymc_marketing.mmm.utility.diversification_ratio.html#pymc_marketing.mmm.utility.diversification_ratio

@williambdean
Copy link
Contributor

Thank you for the PR @saishreyakumar ! Do you need a hand finishing this up?

TeemuSailynoja
TeemuSailynoja requested changes Aug 28, 2025
View reviewed changes
pymc_marketing/mmm/utility.py
@@ -534,12 +534,12 @@ def diversification_ratio(
The Diversification Ratio is calculated as:

.. math::
DR = \frac{\\sum_{i=1}^{n} w_i \\cdot \\sigma_i}{\\sigma_p}
DR = \frac{\sum_{i=1}^{n} w_i \cdot \sigma_i}{\sigma_p} \
Copy link
Contributor

@TeemuSailynoja TeemuSailynoja Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

According to the Sphinx documentation, the equation doesn't need a newline after the math call:

When the math is only one line of text, it can also be given as a directive argument:
.. math:: (a + b)^2 = a^2 + 2ab + b^2

Also, maybe the line should not terminate with \ which might be the cause of the rendering issue @williambdean is seeing..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TeemuSailynoja TeemuSailynoja TeemuSailynoja requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
awaiting response docs Improvements or additions to documentation good first issue Good for newcomers . Doesn't require extensive knowledge of the repo and package MMM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Poor rendering of latex documentation
5 participants
@saishreyakumar @williambdean @TeemuSailynoja @juanitorduz @cetagostini