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

Seaborn plots should fill their output_plot #785

Merged
merged 2 commits into from
Oct 26, 2023
Merged

Conversation

jcheng5
Copy link
Collaborator

@jcheng5 jcheng5 commented Oct 26, 2023

This fixes a bug introduced by #783, which was intended (in part) to allow render.plot code to set plot sizes by using matplotlib figure sizing. That code assumed that only explicit user code would cause the figure size to differ from matplotlib's default figure size (plt.rcParams["figure.figsize"]). But that's not true for seaborn, which likes to set its own sizes, which we then misidentified as being a high priority size directive that we should obey.

https://seaborn.pydata.org/tutorial/function_overview.html#specifying-figure-sizes

This change removes the logic that looks at whether the figure size changed. Instead, render.plot width/height can be set to 0 to cause the matplotlib figure size to be honored. However, we're not going to be emphasizing that in our docs, at least for now.

This fixes a bug introduced by #783, which was intended (in part)
to allow render.plot code to set plot sizes by using matplotlib
figure sizing. That code assumed that only explicit user code
would cause the figure size to differ from matplotlib's default
figure size (plt.rcParams["figure.figsize"]). But that's not true
for seaborn, which likes to set its own sizes, which we then
misidentified as being a high priority size directive that we
should obey.

https://seaborn.pydata.org/tutorial/function_overview.html#specifying-figure-sizes

This change removes the logic that looks at whether the figure
size changed. Instead, render.plot width/height can be set to 0
to cause the matplotlib figure size to be honored. However, we're
not going to be emphasizing that in our docs, at least for now.
@jcheng5 jcheng5 marked this pull request as ready for review October 26, 2023 22:25
@jcheng5 jcheng5 requested a review from wch October 26, 2023 22:25
@jcheng5 jcheng5 merged commit 1b79379 into main Oct 26, 2023
24 checks passed
@jcheng5 jcheng5 deleted the fix-seaborn-plot-sizing branch October 26, 2023 23:19
schloerke added a commit that referenced this pull request Nov 2, 2023
* main: (56 commits)
  Add actions step for nightly reporting -> Testrail (#774)
  Remove unused entry in manifest
  Bump version to 0.6.0.9000
  Use latest htmltools
  Bump version to v0.6.0; Rearrange news to have API changes last
  Tweaks from testing
  Fix default width/height with implicit plot output (#792)
  Update deps (#794)
  Remove deprecated 'name' parameter from `Outputs` (#791)
  api(ui): Drop `toggle_` methods. Consolidate update accordion methods. Stronger typing for `layout_sidebar(sidebar)` and `page_sidebar(sidebar)` (#788)
  bug(sidebar): Revert sidebar icon back to chevron (#789)
  For `input_action_button`, default to having whitespace around button (#758)
  Remove output from template app (#775)
  Add output_args and suspend_display decorators (#786)
  Update value_box; Update to bootstrap 5.3; Update htmldeps (#772)
  tests: add sidebar test (#787)
  Seaborn plots should fill their output_plot (#785)
  Kwargs to uvicorn run (#780)
  Add width and height arguments to `@render.plot` (#783)
  gitgnore dist/
  ...
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.

2 participants