Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add quantile feature to forestplot ridgeplot #1047

Merged
merged 7 commits into from
Feb 13, 2020

Conversation

percygautam
Copy link
Contributor

@percygautam percygautam commented Feb 5, 2020

Description

Fixes #756 . Add new feature quantiles to ridgeplot of forestplot.

Checklist

  • Does the PR follow official
    PR format?
  • Has included a sample plot to visually illustrate the changes? (only for plot-related functions)
  • Is the new feature properly documented with an example?
  • Is the code style correct (follows pylint and black guidelines)?
  • Is the change listed in changelog?

@percygautam
Copy link
Contributor Author

percygautam commented Feb 5, 2020

Currently implemented for matplotlib. It looks something like:

Screenshot 2020-02-06 at 1 06 16 AM

There is a problem of having step size too small in plots. ax.fill_between will inverse fill between x[i] and x[i+1], this cause the quantile line to be very thin. (almost disappear in more dense step sized plots like defs Italy). How can we clearly show these quantiles?

@aloctavodia
Copy link
Contributor

As a workaround you can stretch the figure along the x-axis, something like figsize=(14, 4). Another solution is to re-think how we display quantiles for kdes. Maybe instead of not filling the area we could use two alternating shades (using two different alpha values) ¿?

@percygautam
Copy link
Contributor Author

@aloctavodia I tinkered a little with figsize generation, and below size seems okayish:

    if figsize is None:
        if ridgeplot_quantiles is None:
            figsize = (min(12, sum(width_ratios) * 2), plot_handler.fig_height())
        else:
            figsize = (min(12, sum(width_ratios) * 5), plot_handler.fig_height() * 1.2)

Graph looks like this :
Screenshot 2020-02-11 at 2 34 41 AM

About alternate shades for quantiles, Should I add that also in this? Should alternate shades also be added to normal kdeplot with quantiles?

@aloctavodia
Copy link
Contributor

Not sure, but I guess a good solution is to work on this first and after merge work on a separated PR for the shade feature. For the next PR we may even think on a general function that is called by the kdeplot and the ridgeplot.

@aloctavodia
Copy link
Contributor

I do not think the function should change the figsize automatically, the user should be in control of the figsize

@percygautam
Copy link
Contributor Author

Okay, Let's get this one done first. I have changed the figsize for ridgeplot_quantiles, if it is not explicitly defined by the user. It's creating a figure of (12, 5.2). Is this correct or should we specify some different default figsize for quantiles plot?

@percygautam
Copy link
Contributor Author

percygautam commented Feb 12, 2020

I have added the quantile feature to bokeh ridgeplot, and changed the default figsize to clearly show the quantiles (for [0.2, 0.8]):
Screenshot 2020-02-12 at 4 42 36 PM

Also, The figsize of normal ridgeplot (without quantiles) is not normalized, making the bokeh figure to look cluttered. Should I change that also (just for ridgeplot)?

@@ -58,7 +59,10 @@ def plot_forest(
)

if figsize is None:
figsize = (min(12, sum(width_ratios) * 2), plot_handler.fig_height())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh, sorry I misread your previous example. This is OK

Copy link
Contributor

@ahartikainen ahartikainen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -484,7 +505,7 @@ def treeplot(self, qlist, credible_interval):

def ridgeplot(self, mult, ridgeplot_kind):
"""Get data for each ridgeplot for the variable."""
xvals, yvals, pdfs, colors = [], [], [], []
xvals, yvals, pdfs, pdfs_q, colors = [], [], [], [], []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be

xvals = yvals = pdfs = pdfs_q = colors = []

but it is not that important, so no need to change it

@ahartikainen ahartikainen changed the title [WIP] Add quantile feature to forestplot ridgeplot Add quantile feature to forestplot ridgeplot Feb 13, 2020
@ahartikainen ahartikainen merged commit a07e87a into arviz-devs:master Feb 13, 2020
@percygautam percygautam deleted the ridgeplot_quantiles branch February 17, 2020 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plot_forest extension to allow plot_kde with quantiles
3 participants