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

plot_ppc only plots one legend if there are multiple observed #1540

Closed
MarcoGorelli opened this issue Feb 6, 2021 · 5 comments · Fixed by #1559
Closed

plot_ppc only plots one legend if there are multiple observed #1540

MarcoGorelli opened this issue Feb 6, 2021 · 5 comments · Fixed by #1559

Comments

@MarcoGorelli
Copy link
Contributor

To Reproduce

import seaborn as sns
import pymc3 as pm
import pandas as pd
import arviz as az

iris = sns.load_dataset("iris")
y_s = pd.Categorical(iris["species"]).codes
x_n = "sepal_length"
x_0 = iris[x_n].values

with pm.Model() as lda:
    μ = pm.Normal("μ", mu=0, sd=10, shape=2)
    σ = pm.HalfNormal("σ", 10)
    setosa = pm.Normal("setosa", mu=μ[0], sd=σ, observed=x_0[:50])
    versicolor = pm.Normal("versicolor", mu=μ[1], sd=σ, observed=x_0[50:])
    bd = pm.Deterministic("bd", (μ[0] + μ[1]) / 2)
    trace_lda = pm.sample(1000, return_inferencedata=True)

with lda:
    ppc = pm.sample_posterior_predictive(trace=trace_lda)

az.plot_ppc(az.from_pymc3(posterior_predictive=ppc, model=lda))

image

Expected behavior
I think I'd have expected both subplots to have a legend

Additional context

> az.__version__, pm.__version__
('0.11.1', '3.11.0')
@aloctavodia
Copy link
Contributor

We can change it to optionally have more than one legend, but I think this default makes sense at it reduces redundancy (and the data/ink ratio).

@MarcoGorelli
Copy link
Contributor Author

Sure, but the second subplot is about versicolor (whilst the first one setosa), so is it really redundant to have an extra legend when the first one reads "observed setosa"?

@aloctavodia
Copy link
Contributor

Yes, if you have that information on the x_label. Of course the current behavior is problematic, because we have one legend that is supposed to work for both plots but is referring only to the first. So I suggest to have as default one legend and remove from the legend the name of the variable.

@MarcoGorelli
Copy link
Contributor Author

Agreed, that would be good - I'll work on a PR next week

@aloctavodia
Copy link
Contributor

Thanks @MarcoGorelli!

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 a pull request may close this issue.

2 participants