Skip to content

Update ref_dim in add_reference_lines #254

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

Merged
merged 2 commits into from
May 15, 2025

Conversation

rohanbabbar04
Copy link
Contributor

@rohanbabbar04 rohanbabbar04 commented May 14, 2025

  • Pass ref_dim to references_to_dataset

With the references_to_dataset now updated in this PR, making the fix to support it here.


📚 Documentation preview 📚: https://arviz-plots--254.org.readthedocs.build/en/254/

Copy link
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

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

Given the goal is for this to eventually add text too if requested, I would update the default value for ref_dim in this function too to match the one in arviz-base. It also looks like I forgot to add it to the docstring

@codecov-commenter
Copy link

codecov-commenter commented May 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.96%. Comparing base (62bdc22) to head (c06494d).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #254      +/-   ##
==========================================
+ Coverage   69.30%   70.96%   +1.65%     
==========================================
  Files          41       42       +1     
  Lines        4721     4859     +138     
==========================================
+ Hits         3272     3448     +176     
+ Misses       1449     1411      -38     

☔ 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.

@rohanbabbar04 rohanbabbar04 requested a review from OriolAbril May 14, 2025 09:32
@aloctavodia
Copy link
Contributor

While here, what do you think about renaming add_reference_lines to add_lines?

@rohanbabbar04
Copy link
Contributor Author

While here, what do you think about renaming add_reference_lines to add_lines?

I'm fine with that. We actually kept the word 'reference' in mind because we have ref_dim, and the function's initial purpose was to plot reference lines. However, ideally, it could also be used to plot other vlines/hlines.

@rohanbabbar04 rohanbabbar04 changed the title Minor fix add_reference_lines Update ref_dim in add_reference_lines May 14, 2025
@rohanbabbar04
Copy link
Contributor Author

Should we merge this? We can rename add_reference_lines to add_lines (if we wish to change it) in another PR, since the tests will fail for other PRs until this goes in

@aloctavodia aloctavodia merged commit 2a0fbc2 into arviz-devs:main May 15, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants