Skip to content

Notify Slider observers before requesting a draw in set_val - #32535

Closed
Nacl-HCl wants to merge 1 commit into
matplotlib:mainfrom
Nacl-HCl:fix-slider-set-val-notify-first-32526
Closed

Nacl-HCl wants to merge 1 commit into
matplotlib:mainfrom
Nacl-HCl:fix-slider-set-val-notify-first-32526

Conversation

@Nacl-HCl

Copy link
Copy Markdown

What

Fixes #32526.

Slider.set_val called canvas.draw_idle() before notifying 'changed'
observers, so a callback that updates other artists caused two renders per
event — the first drawn with the previous data. In the reporter's app (four
panels redrawn per slider event) one event cost 97 ms, 49 ms of it the wasted
stale render.

Changes

  • lib/matplotlib/widgets.py — Slider.set_val: notify 'changed'
    observers first, then request the draw when drawon is set. self.val is
    assigned before notification so observers see the new value. A callback's
    own draw_idle() now coalesces with the widget's single draw instead of
    producing a second, stale one. (RangeSlider.set_val has the same shape
    but is left untouched — out of scope of the reported issue.)
  • lib/matplotlib/tests/test_widgets.py — new regression test
    test_slider_set_val_notifies_before_draw: wraps canvas.draw_idle and
    asserts the observer ran before set_val requested its draw.

Verification

  • Reproduced on unpatched main with the issue's scenario (Agg backend):
    the draw was requested with 0 observer events fired; after the fix the
    single draw is requested after the observer ran.
  • New test fails on unpatched code (draw_calls == [0]), passes with the
    fix (draw_calls == [1]).
  • test_widgets.py in full: 135 passed, 1 skipped (all 11 slider cases
    pass; 2 image-comparison tests fail only in wheel installs that lack
    baseline images — unrelated to this change; they pass when run from the
    repo tree).
  • ruff check (repo-pinned v0.15.20): clean on both changed files.

Known limitations

  • The full repo test suite was not run; the change is confined to
    Slider.set_val, and test_widgets.py is fully green.

AI Disclosure

AI tools assisted in the development of this patch; all changes were reviewed and tested by me, and I take responsibility for them.

Slider.set_val called canvas.draw_idle() before notifying 'changed'
observers, so a callback updating other artists triggered a stale extra
render (two renders per event, the first drawn with previous data).

Notify observers first, then request the draw, so the callback's own
draw request coalesces with the widget's.

Fixes matplotlib#32526
@github-actions

Copy link
Copy Markdown

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process.

You can also join us on discourse chat for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide.
Please let us know if (and how) you use AI, it will help us give you better feedback on your PR.

We strive to be a welcoming and open project. Please follow our Code of Conduct.

@github-actions github-actions Bot added first-contribution AgentScan: automated AgentScan classified an account as automated. labels Oct 24, 2026
@Nacl-HCl

Copy link
Copy Markdown
Author

Note on the Python 3.12 on macos-15 failure: it failed after ~90s during environment setup — brew install azure-cli hit curl: (6) Could not resolve host: github.com, so the runner had no network access before any test could run. This looks like runner-side network flakiness, unrelated to this change (a 4-line reorder in Slider.set_val plus one regression test; test_widgets.py passes locally: 135 passed, 1 skipped).

@iccir

iccir commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

I am closing this pull request in accordance with our Policy on Generative AI. The account in question has opened several pull requests across multiple repositories. In addition, AgentScan has detected automation signals from the account.

@iccir iccir closed this Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AgentScan: automated AgentScan classified an account as automated. first-contribution topic: widgets/UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Slider.set_val renders the figure before it notifies its observers, sometimes causing two renders per event

2 participants