Single sided violin plots (enabbling split-violin plots) [WIP] - feedback welcome #1996
Conversation
Changed attribute access from getattr to [] in concat to fix arviz-devs#1993. I have not changed it in lines 2012/2089 and 2017/2094 where "attrs" are handled. But I did test concatenating with variables named "attrs" which seems to work.
The formerly problematic access where variables are indexed within a group with getattr is now using []. In case a group is accessed within an arg (InferenceData object), it remains getattr. There are some other occurrences where groups are used as indices to e.g. dictionaries (e.g. line 1922) where [] access seems correct.
|
Thanks for the PR, sorry I haven't had time to review yet. Will try to get to it soon, probably next week. |
No worries. I'll be quite busy with work-related stuff ~mid next week. Then I will check why the checks don't succeed --- I guess I should run tests locally to find out ... |
There was a problem hiding this comment.
Looks good!
I am afraid I am completely unaware of how plot_violin works currently, I need to run some examples locally at some point. I see you have some comments about defaults in the linked doc with examples, the best I can do for now on that end is no opinion yet.
New features are properly documented (with an example if appropriate)? --> Let me know if needed
I think the first example would be a great addition to the example gallery. I have a labeling suggestion to make only:
import arviz as az
data = az.load_arviz_data("rugby")
labeller = az.labels.MapLabeller(var_name_map={"defs": "atts | defs"})
axs = az.plot_violin(data, var_names = ['atts'], side="left", show=False)
az.plot_violin(data, var_names=['defs'], side="right", labeller=labeller, ax=axs, show=True)
Includes new or updated tests to cover the new feature --> Let me know if needed
Ideally yes. Test for plotting are not very rigorous, but we generally have some parametrization checking different kwargs. See if there is one for plot_violin and add a "side": "left" to one of the cases (no need to add more cases to the test), if there is no general test with kwargs parametrization maybe we can add one. The plot might be broken and look terrible with the test still passing, but if the argument is removed or the processing breaks it can be caught in these tests.
Codecov Report
@@ Coverage Diff @@
## main #1996 +/- ##
==========================================
- Coverage 90.75% 90.73% -0.02%
==========================================
Files 117 117
Lines 12455 12488 +33
==========================================
+ Hits 11303 11331 +28
- Misses 1152 1157 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. |
Co-authored-by: Oriol Abril-Pla <[email protected]>
Co-authored-by: Oriol Abril-Pla <[email protected]>
Description
Added the possibility to plot single-sided violin plots. By plotting the left side for some data and subsequently plotting the right side into the same axes enables the creation of split-violin plots that can be useful for comparing related distributions (e.g. same measurement under different conditions). Cases such continuous vs. discrete violin plots, with and without rugs are handled.
See attached PDF for examples (and another in this tweet https://twitter.com/tuennermann/status/1493909800782020610).
I think it is usable as is, but I mark the PR as WIP because I'm not sure if/what documentation/tests might be required. Please let me know.
Examples: TestSplitHalfViolinPlot.pdf
Checklist