Skip to content

Add function for plotting reference bands #258

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 11 commits into from
May 19, 2025

Conversation

rohanbabbar04
Copy link
Contributor

@rohanbabbar04 rohanbabbar04 commented May 15, 2025

Initial draft for plotting reference bands

Closes #227


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

@codecov-commenter
Copy link

codecov-commenter commented May 15, 2025

Codecov Report

Attention: Patch coverage is 72.83951% with 22 lines in your changes missing coverage. Please review.

Project coverage is 73.04%. Comparing base (827cece) to head (d6dc294).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/arviz_plots/backend/none/__init__.py 50.00% 7 Missing ⚠️
src/arviz_plots/backend/bokeh/__init__.py 60.00% 4 Missing ⚠️
src/arviz_plots/backend/plotly/__init__.py 60.00% 4 Missing ⚠️
src/arviz_plots/backend/matplotlib/__init__.py 62.50% 3 Missing ⚠️
src/arviz_plots/plots/utils.py 93.75% 2 Missing ⚠️
src/arviz_plots/visuals/__init__.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #258      +/-   ##
==========================================
+ Coverage   72.44%   73.04%   +0.60%     
==========================================
  Files          42       43       +1     
  Lines        4921     5020      +99     
==========================================
+ Hits         3565     3667     +102     
+ Misses       1356     1353       -3     

☔ 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 changed the title Add Reference bands Add function for plotting reference bands May 15, 2025
@rohanbabbar04 rohanbabbar04 marked this pull request as ready for review May 16, 2025 14:11
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.

I think we could also add the add_xyz functions to be considered functions worthy of the minigallery. If you are interested in playing with this on the gallery_generator let me know and I can help, otherwise we'll open an issue to handle it later on.

@rohanbabbar04
Copy link
Contributor Author

I think we could also add the add_xyz functions to be considered functions worthy of the minigallery. If you are interested in playing with this on the gallery_generator let me know and I can help, otherwise we'll open an issue to handle it later on.

Hmm...
Sounds good. Should we do it in a new PR? I think it would be better to keep the add_reference_bands in this PR and get it merged. I can work on the minigallery in a separate PR.

@rohanbabbar04 rohanbabbar04 requested a review from OriolAbril May 19, 2025 05:04
Dimensions to reduce unless mapped to an aesthetic.
Defaults to ``rcParams["data.sample_dims"]``
ref_dim : list, optional
Specifies the name of the reference dimension for reference values.
Copy link
Member

Choose a reason for hiding this comment

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

I would also update this to match the behaviour of this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

)

requested_aes = set(aes_map["ref_band"]).difference(plot_collection.aes_set)
*ref_dim, _band_dim = ref_dim
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why pylint is not complaining, maybe the leading underscore. But I think we should do one of the following.

  • use *ref_dim, _ to not store the last dimension as anything
  • use *ref_dim, band_dim and then check that band_dim has length 2 and raise an informative error otherwise (as I think the visual would break or behave weirdly in such case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have added a check for this

@rohanbabbar04 rohanbabbar04 requested a review from OriolAbril May 19, 2025 18:18
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.

ready to merge once the docstring is fixed

@OriolAbril OriolAbril merged commit 7c67381 into arviz-devs:main May 19, 2025
3 checks passed
@rohanbabbar04 rohanbabbar04 deleted the ref_bands branch May 20, 2025 03:37
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.

Add function to plot bands
3 participants