Adding Ridgeplot to Arviz-Plots

imperorrp commented 2 months ago

First commit for adding ridge plot. As per issue #7, the already implemented forest plot structure was taken as base logic for this plot and the conventions for adding a new plot (https://arviz-plots.readthedocs.io/en/latest/contributing/new_plot.html) have been followed. This plot plots kde ridges instead of credible intervals and point estimates.

I also added two top-level arguments to this plot since I thought they might be useful-

rescale_kde (rescales kde by raising the 'y' coord values in the density dataset to a power, set to 1 by default. Using exponent here to amplify the peaks so they're more prominently visible because there's a tendency for kdes to appear very flat when plot_ridge() has a lot of subplots stacked)
plot_ridge_base (boolean to choose whether or not to plot a baseline for each ridge. I think this helps in legibility if the ridges overlap each other or have high peaks, especially if the kde was rescaled)

Also modified the line_xy visual element function to accept an additional y argument (set to None by default) which is added to da.sel(plot_axis='y') if it is not None, so that each ridge is plotted at the correct height corresponding to its labels on the left column. Since the functionality is the same as previously if y is not provided, this ensures the function works where used already (like in plot_dist()) without deprecation.

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

codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 90.68627% with 19 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@6f8b750). Learn more about missing BASE report. Report is 1 commits behind head on main.

Files	Patch %	Lines
src/arviz_plots/plots/ridgeplot.py	90.32%	18 Missing :warning:
src/arviz_plots/visuals/__init__.py	93.75%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #57 +/- ## ======================================= Coverage ? 85.75% ======================================= Files ? 17 Lines ? 1951 Branches ? 0 ======================================= Hits ? 1673 Misses ? 278 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

imperorrp commented 2 months ago

Some examples of the plot output with the centered_eight data.

Without the extra top-level args for rescaling/adding a baseline:

azp.plot_ridge(data)

azp.plot_ridge(data, var_names=["mu", "tau"])

With rescaling:

azp.plot_ridge(data, rescale_kde=0.5)

azp.plot_ridge(data, var_names=["mu", "tau"], rescale_kde=0.5)

azp.plot_ridge(data, rescale_kde=1.5)

azp.plot_ridge(data, var_names=["mu", "tau"], rescale_kde=1.5)

With base line added:

azp.plot_ridge(data, plot_ridge_base=True)

azp.plot_ridge(data, var_names=["mu", "tau"], plot_ridge_base=True)

(Looks a bit cluttered when baselines are added when there's a lot of ridges but for the latter I think helps in seeing the density, especially if some ridges are overlapping.)

imperorrp commented 2 months ago

Renamed the ridge artist to edge and added the face artist. I had to modify the fill_between_y visual element function a bit as well so that the 'y' aesthetic argument passed to it by pc.map is considered and also updated the line_xy visual element with the pattern those other visual element functions use.

Also moved the kde rescaling code to just after statistical computation so that a new dataset face_density can be created and later fed to the fill_between_y function.

About the default color for face- I've just used the same one as for edge for now but with the alpha=0.4 added. Have left the bit about face/edge using the same properties for now as well, for after discussion like you said.

Also added some tests, with style drawn from the existing ones for plot_dist and plot_forest. Tried adding some hypothesis tests but there were issues with that so I haven't committed those changes yet.

Here's how the plots look like now:

When combined==True (by default)

When combined==False

imperorrp commented 1 month ago

Tried adding some hypothesis tests but there were issues with that so I haven't committed those changes yet.

Try manually generating a plot_ridge without the edge and another without the face. The hypothesis test if you copied it from the plot_forest one is failing at least due to that

Yeah I just checked this- since the current code calls the density calculation function only if edge_kwargs is not False, it doesn't work when edge is turned off with plot_kwargs. That might be the reason behind the test failing. I could move the density computation outside to fix this, with perhaps a check for if edge_kwargs or face_kwargs.

imperorrp commented 1 month ago

Modified density calculation to occur if either one of edge_kwargs or face_kwargs is not False, indicating that it is required to be calculated for plotting to be possible. Also added the visual element function changes and hypothesis tests code, which appears to pass now for plot_ridge

arviz-devs / arviz-plots