{{ message }}
Repository navigation
FixedLocator: copy ndarray input instead of aliasing it - #32367
Closed
pranjalm37 wants to merge 1 commit into
Closed
pranjalm37 wants to merge 1 commit into
pranjalm37 wants to merge 1 commit into
Conversation
FixedLocator.__init__ stored locs via np.asarray(locs), which is a no-op for an already-ndarray input -- it returns the same object rather than a copy. Since set_ticks()/set_xticks() build their FixedLocator directly from whatever array the caller passed, any later in-place mutation of that array (e.g. `theta[i] += ...`) moved the axis' already-set tick locations too, silently changing the plot. A plain list input didn't have this problem, since converting a list always produces a fresh array. Use np.array(locs) instead, which copies by default, so FixedLocator owns its own storage regardless of the input type. Fixes matplotlib#32313.
Member
|
Hi @pranjalm37 and thank you for your interest in improving Matplotlib. From your PR description, it does not look like you read the comments on the issue. Please read those comments and say why you think this fix is still necessary in the light of them. |
Author
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
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.

Fixes #32313.
Root cause
FixedLocator.__init__stores its tick locations vianp.asarray(locs):np.asarrayis a no-op when its input is already an ndarray — it returns the same array object, not a copy. SinceAxis.set_ticks()/set_xticks()build aFixedLocatordirectly from whatever array the caller passed in,FixedLocator.locsends up aliasing that array. Any later in-place mutation of it retroactively moves the axis' already-set tick locations:A plain
listinput doesn't show this, since converting a list to an array always produces a fresh one — which is presumably why this went unnoticed for a while.Fix
Use
np.array(locs)instead, which copies by default, soFixedLocatorowns its storage regardless of whether the input was a list or an existing ndarray.Testing
TestFixedLocator.test_locs_is_a_copy_of_ndarray_input, which constructs aFixedLocatorfrom an ndarray, mutates the array in place afterward, and asserts.locsis unaffected. Verified it fails on unpatchedticker.py(fixed.locs is not locs→AssertionError) and passes with the fix.matplotlibwheel with justticker.pyswapped in, since I couldn't build the C extensions from source in my sandbox — no network access for the bundled freetype/etc. fetch): confirmedax.get_xticks()changes after mutating the input array on unpatched code, and stays stable with the fix. Also confirmed a plain-listset_xticks([1, 2, 3])call is unaffected.ruff checkon both changed files — clean (matplotlib'spyproject.tomlonly configuresruff check, notruff format, so I didn't run the formatter).AI disclosure
I used Claude Code to investigate and draft this fix; I reviewed the diagnosis (traced
set_ticks→FixedLocator.__init__→np.asarray), verified the reproduction and fix behavior myself as described above, and reviewed every changed line before opening this PR.🤖 Generated with Claude Code