Skip to content
New issue

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

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

Already on GitHub? # to your account

remove manual setting of 2d kde lims #1939

Merged
merged 5 commits into from
Jan 3, 2022
Merged

remove manual setting of 2d kde lims #1939

merged 5 commits into from
Jan 3, 2022

Conversation

OriolAbril
Copy link
Member

Description

A few lines above we set x_x and y_y using the min and max along each dimension, so this has no visual effect on the plot, as long as you are plotting models independently. I think it is important to remove in order to allow using the kde mode of plot_pair to compare models. Otherwise only the last kde to be plotted is guaranteed to be seen complete, the rest are generally cut or lay completely outside the limits.

Checklist

  • Does the PR follow official PR format?
  • Does the PR include new or updated tests to prevent issue recurrence (using pytest fixture pattern)? No, could think of adding one, not sure if necessary
  • Is the code style correct (follows pylint and black guidelines)?
  • Is the fix listed in the Maintenance and fixes section of the changelog?

@codecov
Copy link

codecov bot commented Nov 16, 2021

Codecov Report

Merging #1939 (3b126d4) into main (e4c9c8e) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1939      +/-   ##
==========================================
- Coverage   91.51%   91.49%   -0.02%     
==========================================
  Files         113      113              
  Lines       12231    12207      -24     
==========================================
- Hits        11193    11169      -24     
  Misses       1038     1038              
Impacted Files Coverage Δ
arviz/plots/backends/bokeh/distplot.py 84.37% <100.00%> (-0.48%) ⬇️
arviz/plots/backends/bokeh/kdeplot.py 95.89% <100.00%> (-0.27%) ⬇️
arviz/plots/backends/bokeh/pairplot.py 86.95% <100.00%> (-0.55%) ⬇️
arviz/plots/backends/matplotlib/distplot.py 92.30% <100.00%> (-0.12%) ⬇️
arviz/plots/backends/matplotlib/kdeplot.py 98.85% <100.00%> (-0.04%) ⬇️
arviz/plots/backends/matplotlib/pairplot.py 91.93% <100.00%> (-0.17%) ⬇️
arviz/plots/plot_utils.py 89.03% <100.00%> (+0.19%) ⬆️

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 e4c9c8e...3b126d4. Read the comment docs.

@OriolAbril OriolAbril marked this pull request as draft November 16, 2021 23:22
@ahartikainen
Copy link
Contributor

is there a possibility to check data limits?

Or does mpl this for us?

@OriolAbril
Copy link
Member Author

both matplotlib and bokeh do that automatically, you can see in the example gallery preview for example how everything looks the same: https://arviz--1939.org.readthedocs.build/en/1939/examples/plot_pair_kde.html or https://arviz--1939.org.readthedocs.build/en/1939/examples/plot_pair_kde_hdi.html

@OriolAbril OriolAbril marked this pull request as ready for review December 10, 2021 03:16
@OriolAbril OriolAbril merged commit ec789c8 into main Jan 3, 2022
@OriolAbril OriolAbril deleted the 2d_kde_lims branch January 3, 2022 19:48
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants