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

Pairplot of two variables with bokeh #1179

Merged
merged 8 commits into from
May 11, 2020
Merged

Pairplot of two variables with bokeh #1179

merged 8 commits into from
May 11, 2020

Conversation

agustinaarroyuelo
Copy link
Contributor

@agustinaarroyuelo agustinaarroyuelo commented May 10, 2020

Description

This PR addresses issue #1169.
bokeh_plot

Checklist

  • Follows official PR format
  • Includes a sample plot to visually illustrate the changes (only for plot-related functions)
  • Code style correct (follows pylint and black guidelines)
  • Changes are listed in changelog

ahartikainen and others added 3 commits May 10, 2020 15:51
…es (#1079)

* add jointplot features into pairplot

* add scatter_kde kind for pairplot

* add point_estimate arguments

* bokeh backend

* fix None argument for color in kdeplot bokeh backend

* run black, pylint and pytest

* remove scatter_kde kind among several other changes

* minor changes

* run pytest

* add plot width and height to backend_kwargs

fix pylint issues

fix hover feature

fix hover feature

minor fixes

* update docstring

run pylint

* update changelog
* Transform colors to hex in plot_khat

* Reformatted with black

* Added hex conversion to khatplot.py and vectorized_to_hex to plot_utils.py

* Black changes

* Added keep_alpha parameter and list comprehension

* Pydocstyle changes

* More pydocstyle changes

* generalised vectorized_to_hex

* Black changes

* Black changes

* Modified khatplot hline_kwargs, vectorized_to_hex and test for vectorized_to_hex

* Black changes

* Rewrote tests and modified vectorized_to_hex

* lint and minor fixes

Co-authored-by: Oriol (ZBook) <oriol.abril.pla@gmail.com>
@codecov
Copy link

codecov bot commented May 10, 2020

Codecov Report

Merging #1179 into master will increase coverage by 0.00%.
The diff coverage is 92.59%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1179   +/-   ##
=======================================
  Coverage   93.11%   93.12%           
=======================================
  Files          94       94           
  Lines        9339     9351   +12     
=======================================
+ Hits         8696     8708   +12     
  Misses        643      643           
Impacted Files Coverage Δ
arviz/plots/backends/bokeh/pairplot.py 85.00% <88.88%> (+0.28%) ⬆️
arviz/plots/khatplot.py 100.00% <100.00%> (ø)
arviz/plots/plot_utils.py 94.57% <100.00%> (+0.15%) ⬆️

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 7177393...2ac10ef. Read the comment docs.

@@ -19,6 +19,7 @@
* New grayscale style. This also add two new cmaps `cet_grey_r` and `cet_grey_r`. These are perceptually uniform gray scale cmaps from colorcet (linear_grey_10_95_c0) (#1164)
* Add warmup groups to InferenceData objects, initial support for PyStan (#1126) and PyMC3 (#1171)

Copy link
Member

Choose a reason for hiding this comment

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

Being picky here. Looks like theres a blank line that can be removed

Copy link
Member

Choose a reason for hiding this comment

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

Is this changelog item correct? It feels like this PR is internally facing readability changes (which is great) but the feature described here ie something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was a duplicated line. Thank you for your helpful comment

@@ -179,7 +185,7 @@ def get_width_and_height(jointplot, rotate):
row_ax.append(None)
else:
jointplot = row == col and numvars == 2 and marginals
rotate = n == 1
rotate = col == 1
Copy link
Member

Choose a reason for hiding this comment

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

Thanks, much easier to read now

ahartikainen and others added 3 commits May 11, 2020 09:05
…es (#1079)

* add jointplot features into pairplot

* add scatter_kde kind for pairplot

* add point_estimate arguments

* bokeh backend

* fix None argument for color in kdeplot bokeh backend

* run black, pylint and pytest

* remove scatter_kde kind among several other changes

* minor changes

* run pytest

* add plot width and height to backend_kwargs

fix pylint issues

fix hover feature

fix hover feature

minor fixes

* update docstring

run pylint

* update changelog
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, after fixing the changelog

CHANGELOG.md Outdated Show resolved Hide resolved
@canyon289
Copy link
Member

Ship it!

@canyon289 canyon289 merged commit 7be3ffd into arviz-devs:master May 11, 2020
@canyon289
Copy link
Member

Ah shoot I merged a little too quickly, which is fine. Next time do a rebase on master so the changes from master dont appear in this PR.

Ill slow down next time, my bad

@canyon289
Copy link
Member

Basically thing change shouldnt have appeared in set of commits

image

@agustinaarroyuelo agustinaarroyuelo deleted the jointplot_bokeh branch May 11, 2020 23:27
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.

6 participants