Cleanup QuiverKey init and deprecate some attributes.#31170
Merged
ksunden merged 1 commit intomatplotlib:mainfrom Mar 6, 2026
Merged
Cleanup QuiverKey init and deprecate some attributes.#31170ksunden merged 1 commit intomatplotlib:mainfrom
ksunden merged 1 commit intomatplotlib:mainfrom
Conversation
Ensure that QuiverKey.vector exists as soon as the constructor exits, rather than waiting for the first draw. Deprecate the `fontproperties`, `labelcolor`, and `verts` attribute (overwriting them would not actually update the underlying artist (except before the first draw) anyways). Also deprecate `kw` (which *could* be updated with effect, but it seems simpler to directly update the underlying artist here too, if really needed; moreover the old version, which mutated `self.Q.polykw` *in-place*, likely led to weird side-effects e.g. if a key is first added to `kw`, a draw is triggered, mutating `self.Q.polykw`, then the key is removed).
timhoffm
approved these changes
Feb 20, 2026
tacaswell
reviewed
Feb 20, 2026
Comment on lines
+378
to
+379
| if False: # self._dpi_at_last_init == self.axes.get_figure().dpi | ||
| return |
Member
There was a problem hiding this comment.
Suggested change
| if False: # self._dpi_at_last_init == self.axes.get_figure().dpi | |
| return |
This code would never run?
Contributor
Author
There was a problem hiding this comment.
I left the old dead check (which was inverted as if True; I just swapped the boolean to dedent the whole block below), because maybe(?) it makes sense to not recompute the vertices at every draw, but only when they get invalidated (which seems to be the original intent). It's not clear why that code was commented out but perhaps we can figure that out another time.
tacaswell
approved these changes
Feb 20, 2026
Member
tacaswell
left a comment
tacaswell
left a comment
There was a problem hiding this comment.
Modulo removing the dead code.
andreas16700
added a commit
to andreas16700/matplotlib
that referenced
this pull request
Mar 16, 2026
andreas16700
added a commit
to andreas16700/matplotlib
that referenced
this pull request
Mar 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ensure that QuiverKey.vector exists as soon as the constructor exits, rather than waiting for the first draw.
Deprecate the
fontproperties,labelcolor, andvertsattribute (overwriting them would not actually update the underlying artist (except before the first draw) anyways). Also deprecatekw(which could be updated with effect, but it seems simpler to directly update the underlying artist here too, if really needed; moreover the old version, which mutatedself.Q.polykwin-place, likely led to weird side-effects e.g. if a key is first added tokw, a draw is triggered, mutatingself.Q.polykw, then the key is removed).Noted while reviewing #31169.
PR summary
PR checklist