You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Aug 1, 2022. It is now read-only.
I was in dire need for statistical annotations and successfully used your library. Thanks a lot for your work, it generally worked great.
Let me still suggest a couple of improvements. I know, this is a potpourri of different things. You can filter the requests one by one and create new issues in case you agree with them.
Your toolbox currently offers two principal features that are blended into one API-function: (1) Compute and format p-values, (2) and create annotated brackets. I'd suggest to factor out a function add_annotated_brackets() that is completely agnostic about any testing and p-values. This function creates the brackets and annotations according to the formatting settings. add_stat_annotation() could then just call add_annotated_brackets().
You could also better factor out statistical testing part, such that the users can interfere with the p-values, filter the brackets of interest etc. (see e.g. Hide non-significant annotations #50), before they actually add the brackets.
Personally, I think there are plenty of other libraries offering statistical testing. Keeping up with with all methodologies (bonferroni, bootstrapping, anova) is possibly difficult. I'd therefore put the primary focus on how to print these p-values (or other information), and make it as easy as possible for the user to create these annotations using the seaborn semantics.
The reason for these suggestion: With the current solution, one has to adjust several default parameters to use custom annotation and to disable the testing. It took me a while figure this out.
add arguments annotat_kws and line_kws that are forwarded to ax.annotate() and ax.plot()/lines.Line2D(), respectively. This will give the user a bit more stylistic freedom (font color, line styles, ...)
ax.annotate(..., clip_on=False, ...) (around Line 590) is wrong, it should be ax.annotate(..., clip_on=(loc=="inside")) I think. At least the setting is not consistent with the ax.plot() command a couple of lines earlier. If the stats annotation falls outside plt.ylim(), the bracket line is clipped, but not the annotation text.
...but maybe you want to leave the clipping options to the user altogether. You don't know exactly how the axes are used and fixed clip settings can mess up things.
Support different types of brackets (permit the user to pass a line path as template that is scaled and transformed)
Support inverted annotation brackets (from below)
In add_annotated_brackets() permit to set or adjust the y-locations of the brackets freely. The default choice to use max(y)+margin is good. But it might be useful to adjust this (e.g. have all brackets at same y).
You certainly know that it is not recommended to use private stuff from other libraries... On the other hand, I can completely understand why you decided to use seaborn's _Plotters, nevertheless. To reverse-engineer the seaborn plots from its Artists would be a nightmare. I recently asked the seaborn crew / Michael Waskom why not to extend seaborn by an annotation infrastructure. See here for the feature request and Michael's response.
The text was updated successfully, but these errors were encountered:
I have to move on and work on my stuff now. But I hope this feedback is still somehow useful. In case it is not, I don't mind if you close the issue. Thanks again for your work and initiative.
Following up on that: It would be nice to have the option for additional annotations, since the framework to obtain the correct position is already in place. What I'm missing is a function to annotate each box with a small textbox, e.g. to add sample sizes to the boxes.
Thanks a lot @normanius for all the suggestions. I agree indeed with most of them, in particular the factoring out to separate the plotting of annotations and the computation of statistical tests. I think it is better to break down and group all these comments into separate issues. If you agree I will create new issues and quote your comments.
I was in dire need for statistical annotations and successfully used your library. Thanks a lot for your work, it generally worked great.
Let me still suggest a couple of improvements. I know, this is a potpourri of different things. You can filter the requests one by one and create new issues in case you agree with them.
add_annotated_brackets()
that is completely agnostic about any testing and p-values. This function creates the brackets and annotations according to the formatting settings.add_stat_annotation()
could then just calladd_annotated_brackets()
.annotat_kws
andline_kws
that are forwarded toax.annotate()
andax.plot()
/lines.Line2D()
, respectively. This will give the user a bit more stylistic freedom (font color, line styles, ...)ax.annotate(..., clip_on=False, ...)
(around Line 590) is wrong, it should beax.annotate(..., clip_on=(loc=="inside"))
I think. At least the setting is not consistent with theax.plot()
command a couple of lines earlier. If the stats annotation falls outsideplt.ylim()
, the bracket line is clipped, but not the annotation text.add_annotated_brackets()
permit to set or adjust the y-locations of the brackets freely. The default choice to use max(y)+margin is good. But it might be useful to adjust this (e.g. have all brackets at same y).You certainly know that it is not recommended to use private stuff from other libraries... On the other hand, I can completely understand why you decided to use seaborn's
_Plotters
, nevertheless. To reverse-engineer the seaborn plots from its Artists would be a nightmare. I recently asked the seaborn crew / Michael Waskom why not to extend seaborn by an annotation infrastructure. See here for the feature request and Michael's response.The text was updated successfully, but these errors were encountered: