refactor kde-related functions and small fixes #2191
Conversation
pymc3/plots/artists.py
Outdated
| @@ -157,3 +137,5 @@ def set_key_if_doesnt_exist(d, key, value): | |||
| display_ref_val(ref_val) | |||
| if rope is not None: | |||
| display_rope(rope) | |||
|
|
|||
There was a problem hiding this comment.
Thanks, I will fix it.
|
This looks great, thanks @aloctavodia! |
| ax.fill_between(x, density, alpha=alpha) | ||
|
|
||
| for label, value in series: | ||
| kdeplot(value, label=label, alpha=alpha, shade=True, ax=ax, |
There was a problem hiding this comment.
do we want to propagate the return values?
There was a problem hiding this comment.
Thanks for the comment. Not sure if I am really understanding you. I think this is what we always do, isn't it?
There was a problem hiding this comment.
yeah, sorry, this is fine (ax and fig get updated in place here)
|
this is awesome. looks good to merge from me, but I can wait if you want to tidy the formatting first? |
|
I think all the cosmetic changes are made and this is ready to merge. |
|
NOOOOO! You removed the kde2plot, I was using it :( .... |
|
@hvasbath what for? |
|
You can just copy&paste it to your own code, but it wasn't used by pymc3. |
|
Yes I will do that. I produce plots of trade-offs between variables like this:
|
|
That's a handsome plot. |
|
If you think it will be useful I can edit my function a little and send a PR. There is absolutely no need to add another dependency for a simple thing like this. If you want to have some covariance ellipses included on top of the true data, we can also include that ... |
|
@hvasbath I think it could be useful. Sorry I did not explain well myself I was not suggesting adding a dependency (I agree it is not necessary) I just shared the link for reference, just to see what others are doing. Your plot is very nice as is. |
|
Thanks for the link @aloctavodia - It looks more practical to displaying lots of parameters, as the pairplot (I use the one in seaborn) can be incredibly slow. |
kdeplot.pywere not used. I remove both of them and create a newkdeplotfunction. Which is used byenergyplotand byplotposterior. Besides, this new function can be used to plot any 1D numpy array. Usually I do this withkdeplotfrom seaborn, but is annoying (and some times confusing to others) that seaborn do not respect boundaries while PyMC3 does.fast_kdefromutils.pytokdeplot.py.kde2plot_opfunction insideartistics.plotposteriorkde plot was not working becausefigsizewas incorrectly passed toax.plot()energyplotthe default value ofalpha=0.35, but it was actuallyalpha=0.5(changed to 0.35)energyplotby list of tuples in order to guarantee the same order of the plotted data.