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

Calculate coordinate mapping after drawing figure #999

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

has2k1
Copy link
Contributor

@has2k1 has2k1 commented Jan 12, 2024

To calculate accurate coordinate mappings for matplotlib (& plotnine) plots
in display coordinates, the figure should know the location of it's subplots.

The location determined by the subplot parameters (subplot_params) and they
come from one or more of these sources, listed from lowest to highest priority:

  1. rcParams
  2. plt.subplots(..., gridspec_kw={..., **subplot_params})
  3. fig.subplot_adjust(**subplot_params)
  4. layout engine

The layout engine has the last say as it dynamically calculates subplot_params
and calls fig.subplot_adjust() to set them.

There are two things to consider:

  1. Only layout engines that are "adjust_compatible" calculate subplot_params
  2. Matplotlib runs the layout engine only when drawing the figure

This PR ensures that there is an "adjust_compatible" layout engine and the plot
is drawn/saved before the coordinate mappings are calculated.

closes has2k1/plotnine#738

To calculate accurate coordinate mappings for matplotlib (& plotnine) plots
in display coordinates, the figure should know the location of it's subplots.

The location determined by the subplot parameters (subplot_params) and they
come from one or more of these sources, listed from lowest to highest priority:

  1. rcParams
  2. plt.subplots(..., gridspec_kw={..., **subplot_params})
  3. fig.subplot_adjust(**subplot_params)
  4. layout engine

The layout engine has the last say as it dynamically calculates subplot_params
and calls fig.subplot_adjust() to set them.

There are two things to consider:

  1. Only layout engines that are "adjust_compatible" calculate subplot_params
  2. Matplotlib runs the layout engine only when drawing the figure

This PR ensures that there is an "adjust_compatible" layout engine and the plot
is drawn/saved before the coordinate mappings are calculated.

closes has2k1/plotnine#738
@wch
Copy link
Collaborator

wch commented Jan 17, 2024

UPDATE: Disregard this comment below -- it looks like I had problems checking out the branch, and so I was still using main for testing this.

I think I still have problems when using this change.

In the screenshot below, notice that for the brush, the xmin/xmax/ymin/ymax are noticeably different from the actual bounds of the rectangle. This is more noticeable if you use a wide window.

image

This is using the code from https://shinylive.io/py/examples/#basic-plot-interaction (but running locally with regular shiny, not shinylive)

@wch wch merged commit c3bb9ff into posit-dev:main Jan 17, 2024
12 of 24 checks passed
@wch
Copy link
Collaborator

wch commented Jan 17, 2024

Thank you!

schloerke added a commit that referenced this pull request Jan 19, 2024
* main: (26 commits)
  api!: Merge RendererBase class into Renderer (#1032)
  chore(render.display): Improve error message (#1020)
  `express.ui.page_opts(title = ...)` now always generates a header (#1016)
  api!: `Renderer.auto_output_ui()` drops `id` arg. Make `RendererBase.output_id` a non-namespaced value. (#1030)
  fix(page_sidebar): Add semicolon to end style declaration (#1027)
  chore: Remove experimental from app (#1028)
  chore: Expose `render.renderer.RendererBaseT` and do not require `| None` when defining Renderer types (#1026)
  bug: Restore legacy renderers while packages transition (#1023)
  Update deploy test apps to use render.code
  Update changelog
  Provide useful message in Express when `input` was not imported (#994)
  Calculate coordinate mapping after drawing figure (#999)
  Remove `express.ui.output_*` functions, add `shiny.express.render` (#1018)
  fix: Do not allow for renderer's to be given to reactive effects (#1017)
  Truncate the requirements.txt file before deploys (#998)
  Update changelog
  Fixes for flake8 (#1012)
  Pin starlette version below 0.35 (#1009)
  Remove shiny express warning
  Cause RecallContextManagers to run when used without `with` (#992)
  ...
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.

Problems with coordinate mapping computation in Shiny
2 participants