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

Removed repetitive variable names from forest plots of multivariate variables #1527

Merged
merged 5 commits into from
Jan 29, 2021
Merged

Conversation

fonnesbeck
Copy link
Contributor

@fonnesbeck fonnesbeck commented Jan 28, 2021

Forest plots of multivariate variables currently prepend the variable name to each element label, which adds a lot of clutter to the plot. This removes the variable names from all but the first element.

Can add this feature to other plots as needed, either here or in a separate PR.

Not sure what sort of test would be relevant here, if any.

Closes #1524

@AlexAndorra AlexAndorra added Enhancement Improvements to ArviZ Feature Request New functionality requests from users labels Jan 28, 2021
Copy link
Contributor

@AlexAndorra AlexAndorra left a comment

Choose a reason for hiding this comment

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

This looks great, thanks @fonnesbeck ! I don't think we need this for more plots -- or maybe plot_parallel ? Curious what the others think.
The test failure is weird, looks like a problem with Azure

@ahartikainen
Copy link
Contributor

How does this look in the case were there are no verbose coords?

@fonnesbeck
Copy link
Contributor Author

@ahartikainen if the variable is not a multivariate random variable, then it uses the variable name. The only change should be in the multivariate case.

@fonnesbeck
Copy link
Contributor Author

@AlexAndorra yes, they look like resource provisioning errors.

@fonnesbeck
Copy link
Contributor Author

Actually, I noticed a bug with multiple non-combined chains:

Screen Shot 2021-01-28 at 1 50 48 PM

Don't merge until I fix this.

@OriolAbril
Copy link
Member

Restarting azure and now it seems to run without problem

@codecov
Copy link

codecov bot commented Jan 28, 2021

Codecov Report

Merging #1527 (a3e126c) into main (238d9bb) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1527      +/-   ##
==========================================
+ Coverage   91.77%   91.79%   +0.01%     
==========================================
  Files         105      105              
  Lines       11330    11342      +12     
==========================================
+ Hits        10398    10411      +13     
+ Misses        932      931       -1     
Impacted Files Coverage Δ
arviz/plots/backends/bokeh/forestplot.py 95.09% <100.00%> (+0.40%) ⬆️
arviz/plots/backends/matplotlib/forestplot.py 98.07% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 238d9bb...a3e126c. Read the comment docs.

@fonnesbeck
Copy link
Contributor Author

OK, fixed. A multi-trace multivariate forest plot now looks like this:

Screen Shot 2021-01-28 at 6 58 29 PM

whereas a forest plot of three univariate variables looks like this:

download

Copy link
Contributor

@aloctavodia aloctavodia left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@AlexAndorra AlexAndorra left a comment

Choose a reason for hiding this comment

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

Thanks @fonnesbeck 🥳

@AlexAndorra AlexAndorra merged commit 1f3b8fd into arviz-devs:main Jan 29, 2021
@OriolAbril OriolAbril mentioned this pull request Feb 19, 2021
12 tasks
utkarsh-maheshwari pushed a commit to utkarsh-maheshwari/arviz that referenced this pull request May 27, 2021
…ariables (arviz-devs#1527)

* Removed repetitive variable names in forest plots for mutivariate variables

* Removed errant indents

* Applied Black formatting

* Added changes to CHANGELOG

* Fixed multi-chain labelling bug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improvements to ArviZ Feature Request New functionality requests from users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variable names in element labels for plot_forest should be optional
5 participants