Skip to content

Explicit index support in PlotCollection.map #241

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 15 commits into from
May 15, 2025
Merged

Conversation

OriolAbril
Copy link
Member

@OriolAbril OriolAbril commented May 5, 2025

See the compose your own plot page for proof of concept.

Closes #237 and should unblock #149 (yet not fix it though as we'll still need to handle the kde/ecdf... computation through groupby for that to work completely).

Also closes #257


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

@OriolAbril OriolAbril force-pushed the xarray_plot_kwargs branch 2 times, most recently from 96775f3 to 96b027f Compare May 12, 2025 14:14
@codecov-commenter
Copy link

codecov-commenter commented May 12, 2025

Codecov Report

Attention: Patch coverage is 75.12821% with 97 lines in your changes missing coverage. Please review.

Project coverage is 72.44%. Comparing base (544e7f1) to head (5d392a4).

Files with missing lines Patch % Lines
src/arviz_plots/plot_collection.py 83.40% 39 Missing ⚠️
src/arviz_plots/backend/none/__init__.py 29.16% 17 Missing ⚠️
src/arviz_plots/backend/plotly/__init__.py 44.82% 16 Missing ⚠️
src/arviz_plots/backend/bokeh/__init__.py 52.17% 11 Missing ⚠️
src/arviz_plots/visuals/__init__.py 83.33% 8 Missing ⚠️
src/arviz_plots/plots/combine.py 33.33% 4 Missing ⚠️
src/arviz_plots/backend/matplotlib/__init__.py 80.00% 1 Missing ⚠️
src/arviz_plots/plot_matrix.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #241      +/-   ##
==========================================
+ Coverage   71.02%   72.44%   +1.41%     
==========================================
  Files          42       42              
  Lines        4860     4921      +61     
==========================================
+ Hits         3452     3565     +113     
+ Misses       1408     1356      -52     

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

@OriolAbril OriolAbril linked an issue May 13, 2025 that may be closed by this pull request
@OriolAbril OriolAbril marked this pull request as ready for review May 13, 2025 12:18
@OriolAbril
Copy link
Member Author

I ended up extending the behaviour to coordinate values and we internally call set_xindex and use dim_to_idx if needed. That being said, I haven't checked how expensive it is but it is definitely better to call set_xindex once, not every time .map is called or a subset of a kwarg requested so we'll need to document that somehow.

Faceting and aes now work from coordinate values. https://arviz-plots--241.org.readthedocs.build/en/241/tutorials/compose_own_plot.html has a demonstration of the behaviour (or will as the rtd build is still in progress as I write this). I have also solved a couple smaller issues in the process, mainly the fn(da, target, backend, ...) to fn(da, target, ...) and renamed all instances of "chart" to "figure".

Needs more testing and documentation (existing ones have been fixed/updated as necessary), but if there are incoming PRs it might be better to merge and add them in a follow up PR to avoid git conflicts.

@OriolAbril
Copy link
Member Author

Legends also need to be fixed to adapt to the new aesthetics logic

@OriolAbril
Copy link
Member Author

Legends fixed, and if I say so myself the new structure allows for much better legends without as much complexity

@OriolAbril OriolAbril force-pushed the xarray_plot_kwargs branch from 5956934 to e6cddeb Compare May 15, 2025 20:16
@OriolAbril OriolAbril merged commit 827cece into main May 15, 2025
3 checks passed
@OriolAbril OriolAbril deleted the xarray_plot_kwargs branch May 15, 2025 20:45
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.

Bayes factor plot is not showing the value of the BF Add ignore_aes="all" option in .map Modify call signature of functions used with .map
3 participants