Added plot_bf() function for bayes_factor in arviz-plots #158
Conversation
|
I have created this PR incorporating all suggested changes. Let me know if any changes are needed. Thank you |
src/arviz_plots/plots/bfplot.py
Outdated
| pc_kwargs=pc_kwargs, | ||
| ) | ||
|
|
||
| if ref_val == 0: |
There was a problem hiding this comment.
ref_val could take any real value.
src/arviz_plots/plots/bfplot.py
Outdated
| pc_kwargs=pc_kwargs, | ||
| ) | ||
|
|
||
| if ref_val == 0: |
There was a problem hiding this comment.
| if ref_val == 0: | |
| if ref_val is not False: |
src/arviz_plots/plots/bfplot.py
Outdated
| x=ref_val + 13, | ||
| y=max(ref_vals["prior"], ref_vals["posterior"]) * 1.7, |
There was a problem hiding this comment.
Not sure these magic numbers will generally work. See for instance https://arviz-plots--158.org.readthedocs.build/en/158/gallery/plot_bf.html for plotly and bokeh backends
There was a problem hiding this comment.
Hey, As the annotation was coming in between the line while plotting. I added this value to adjust the position of the annotation.
Can you suggest if any better way exists?
There was a problem hiding this comment.
I understand that, but as you can see, this solution does not work for the example.
Other solution could be:
- Find a way to position the text using a relative frame-reference
- plot it at max(value_at_posterior, value_at_prior), but just one number (the BF10 or BF01), this is the same as before but because we have just one number it could be less crowed. Probably this is the best not only in this case but in general
- Use the legend
- Use the title.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #158 +/- ##
==========================================
+ Coverage 75.93% 76.28% +0.35%
==========================================
Files 30 32 +2
Lines 3611 3741 +130
==========================================
+ Hits 2742 2854 +112
- Misses 869 887 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Heyy @aloctavodia, I have added a new func called |
|
Most of the code is duplicated from add legend. A better approach may be to add an argument to the existing function instead of duplicating it. Also better to reduce the amount of text. A simpler approach will be to use anotate text, and just reduce the amount of information, adding the value of one of the bayes factors should be enough if the docs explain wich value is the one plotted |
The main issue with the annotated text approach is that the BF info appears in the middle of the plot. Since the legend approach works only with Matplotlib, I can:
|
Hey, as per your suggestion, I have added a new argument to the existing add_legend() function to adjust the Bayes factor values. Let me know if any further modifications are needed. |
| @@ -0,0 +1,23 @@ | |||
| """ | |||
There was a problem hiding this comment.
Bayes factors are not a method for inference diagnostics. They are a tool for model comparison.
There was a problem hiding this comment.
Please fix this. You need to move this file to a different folder
src/arviz_plots/plots/bfplot.py
Outdated
| @@ -0,0 +1,210 @@ | |||
| """Contain functions for Bayesian Factor plotting.""" | |||
There was a problem hiding this comment.
| """Contain functions for Bayesian Factor plotting.""" | |
| """Contain functions for Bayes Factor plotting.""" |
PiyushPanwarFST
force-pushed
the
feature
branch
2 times, most recently
from
ce55030 to
8c3ccb7
Compare
Implemented the plot_bf() function in arviz-plots. Added test cases to validate the functionality of plot_bf(). Introduced a new text_only argument in the add_legend() function. Included plot_bf() in the documentation. Incorporated feedback recieve so far. Resolved all the linter(pylint) errors. Signed-off-by: PiyushPanwarFST <[email protected]>
Implemented the
plot_bf()function inarviz-plots.Added a test case to validate the functionality of
plot_bf().Included
plot_bf()in the documentation.Incorporated feedback received so far.
Resolved all the linter(pylint) errors.
📚 Documentation preview 📚: https://arviz-plots--158.org.readthedocs.build/en/158/