Skip to content

Fix Line3D.draw requiring .shape on array-like coordinates - #32128

Open
thc1006 wants to merge 3 commits into
matplotlib:mainfrom
thc1006:fix/line3d-shape-on-array-like
Open

Fix Line3D.draw requiring .shape on array-like coordinates#32128
thc1006 wants to merge 3 commits into
matplotlib:mainfrom
thc1006:fix/line3d-shape-on-array-like

Conversation

@thc1006

@thc1006 thc1006 commented Jul 28, 2026

Copy link
Copy Markdown

PR summary

Closes #32127.

Line3D.set_data_3d documents its parameters as array-like, checks only that each argument is iterable, and stores them as given:

for name, xyz in zip('xyz', args):
    if not np.iterable(xyz):
        raise RuntimeError(f'{name} must be a sequence')
self._verts3d = args

draw then read self._verts3d[0].shape, so a list raised AttributeError: 'list' object has no attribute 'shape'.

fig = plt.figure()
ax = fig.add_subplot(projection="3d")
(line,) = ax.plot([0.0], [0.0], [0.0])
line.set_data_3d([float("nan")], [float("nan")], [float("nan")])
fig.canvas.draw()

The access lives inside the invalid-scale masking branch, so a finite list draws and only an invalid coordinate reaches it. That is why it went unnoticed.

The requirement arrived in two steps. d18c52b introduced _verts3d[0].shape behind if self._axlim_clip:, so it applied only to lines that had opted into clipping. 629d396 then added _scale_invalid_mask, which runs unconditionally, and changed the guard to if np.any(scale_mask):. Nothing about set_data_3d changed; the block consuming what it stores did. 3.10.9 has neither and draws all of these.

NaN is not the only trigger. On 3.11.1 with lists in _verts3d, AttributeError also comes from inf, from a value <= 0 on a log scale, and from an out-of-view point with axlim_clip=True. Tuples fail the same way. The log-scale row is the awkward one: _scale_invalid_mask was added so that data outside a scale's valid domain is ignored rather than breaking the plot, and here it raises instead.

np.shape accepts both, and unlike converting in the setter it leaves what get_data_3d() returns unchanged, so a caller that reads back what it passed still gets it.

PR checklist

  • Has pytest style unit tests, and pytest passes
  • New features are documented, with examples if plot related — n/a, this is a bug fix with no API change
  • Documentation is sphinx and numpydoc compliant — n/a
  • Added an entry to doc/users/next_whats_new/ if major new feature — n/a
  • Documented in doc/api/next_api_changes/ if API changed — no API change; this makes the documented contract hold

Verification

Against a released 3.11.1 with only art3d.py replaced, so the comparison is like for like:

new test full test_art3d.py
3.11.1 as released 1 failed
with this change 1 passed 11 passed

Note on a sibling occurrence

Path3DCollection.do_3d_projection has the same pattern one screen down:

mask = np.broadcast_to(mask, (len(self._offsets3d), *self._offsets3d[0].shape))

I left it alone. _offsets3d is private and its public writer routes through juggle_axes(xs, ys, np.atleast_1d(zs), zdir) with xs, ys coming from get_offsets(), so it holds arrays; ax.scatter with a NaN draws fine. Assigning _offsets3d a tuple of lists directly does reproduce the same error, so it is the same defect, just not one reachable through public API. Happy to include it here if you would rather the two were consistent.

I checked the other users of _scale_invalid_mask and _viewlim_mask the same way, driving each through its public entry point with list input and a triggering coordinate: Line3DCollection, Poly3DCollection, ax.scatter, ax.text, plot_surface and ax.plot all draw, because each converts to arrays on the way in. The two Line3D setters are the exception, and they are the documented way to update an existing 3D line in place. Details in #32127.

How this was found

Downstream code updating a rocket marker each frame, written against the advice in #22308 to use set_data([a], [b]) and set_3d_properties([c]). That produces exactly this state, and it drew on 3.10 and raised on 3.11 the moment a coordinate went non-finite, which happens before the object being plotted has a position.


Update: the test was covering one way in, of three

set_data_3d with a list of NaN on a linear scale was the only case. The masking branch is entered when a coordinate is non-finite, when it is outside the active scale's domain, or when it is outside the view limits with axlim_clip enabled. The last two need no non-finite value at all: 0.0 on a log axis reaches it, and so does a finite in-domain point that is simply off screen. My first commit message said only a non-finite coordinate could get there, which was wrong.

Parametrized over list and tuple, and over five ways in: NaN, inf, a finite value outside a log domain, a finite value masked by axlim_clip, and a multi-vertex line where only one vertex is masked.

against released 3.11.1, art3d.py only result
before this change 10 failed
after 10 passed
rest of test_art3d.py 20 passed

Backport

3.11.1 is affected, so this wants a v3.11.x backport once it lands. Flagging rather than labelling, since I cannot set milestones.

Two findings from the same audit, filed separately

Neither belongs in this PR, and neither blocks it.

#32129 is the one I would look at next. ScaleBase.val_in_range calls limit_range_for_scale with an array, and when a third-party scale is written for scalars, which is the shape LogScale.limit_range_for_scale itself uses, the except (TypeError, ValueError) fallback returns "nothing is in range". _scale_invalid_mask negates that, so every point on the axis is masked and every 3D artist silently disappears with ordinary finite data. Measured: line 0/4 and scatter 0/4 finite points after projection, no warning, no exception. Silent deletion seems worse than the crash this PR fixes.

Line3D.set_data_3d also accepts two or four coordinate sequences, because zip('xyz', args) truncates silently. The failure then surfaces at draw time as TypeError: _scale_invalid_mask() missing 1 required positional argument, which names an internal function at the user. Pre-existing and unrelated to the masking change; I have not filed it separately, and can if it is wanted.

set_data_3d documents its parameters as array-like and checks only that
each is iterable, storing them as given. draw then read
self._verts3d[0].shape, so a list raised AttributeError.

The access sits inside the invalid-scale masking branch, so a finite
list draws and only a non-finite coordinate reaches it. That is why
this survived until 3.11 added the branch.

Use np.shape, which leaves what get_data_3d returns unchanged.

Closes matplotlib#32127
Copilot AI review requested due to automatic review settings July 28, 2026 08:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@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.

thc1006 added 2 commits July 28, 2026 16:24
The test covered one way in: a list of all-NaN coordinates on a linear
scale. The masking branch is reached three ways, and the earlier commit
message said only a non-finite coordinate could get there, which is
wrong.

It is entered when a coordinate is non-finite, when it is outside the
active scale's domain, or when it is outside the view limits and
axlim_clip is enabled. The last two need no non-finite value at all: a
plain 0.0 on a log axis reaches it, and so does a finite in-domain point
that is simply off screen.

Parametrized over list and tuple, and over five ways in: NaN, inf, a
finite value outside a log domain, a finite value masked by axlim_clip,
and a multi-vertex line where only one vertex is masked. Against a
released 3.11.1 with only art3d.py replaced, all ten fail before this
change and pass after, and the rest of test_art3d.py stays green.
@thc1006

thc1006 commented Jul 28, 2026

Copy link
Copy Markdown
Author

The Azure Windows py313 leg is red on test_backend_tk.py::test_dpi_change_triggers_resize, which times out waiting on a GUI subprocess:

subprocess.TimeoutExpired: Command '[... test_dpi_change_triggers_resize()]' timed out after 60 seconds
Failed: Subprocess timed out
==== 1 failed, 8927 passed, 1546 skipped, 29 xfailed in 1275.14s ====

That is the Tk backend under a 60 second subprocess deadline, not anything this PR touches. The diff is mpl_toolkits/mplot3d/art3d.py and its test file, and the 3D tests pass on that same run. Same build also logged repeated HTTP Error 502 fetching the freetype tarball, so the machine was having a slow time of it generally.

Leaving it rather than pushing an empty commit, since a re-run is yours to trigger and I would rather not churn the branch.

@scottshambaugh

scottshambaugh commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

This reads to me as completely AI-generated, and it's not clear if this or coercing _verts3d to an array is a better solution. Please write up your thoughts on this in your own voice without using AI, except for direct word-to-word translation. Marking for autoclose until that's done.

@scottshambaugh scottshambaugh added ai-contribution PRs that are AI generated without a human in the loop status: autoclose candidate PRs that are not yet ready for review and may be automatically closed in two weeks labels Jul 28, 2026
@github-actions

Copy link
Copy Markdown

⏰ This pull request might be automatically closed in two weeks from now.

Thank you for your contribution to Matplotlib and for the effort you have put into this PR. This pull request does not yet meet the quality and clarity standards needed for an effective review. Project maintainers have limited time for code reviews, and our goal is to prioritize well-prepared contributions to keep Matplotlib maintainable.

Matplotlib maintainers cannot provide one-to-one guidance on this PR. However, if you ask focused, well-researched questions, a community member may be willing to help. 💬

To increase the chance of a productive review:

As the author, you are responsible for driving this PR, which entails doing necessary background research as well as presenting its context and your thought process. If you are a new contributor, or do not know how to fulfill these requirements, we recommend that you familiarize yourself with Matplotlib's development conventions or engage with the community via our Discourse or one of our meetings before submitting code.

If you substantially improve this PR within two weeks, leave a comment and a team member may remove the status: autoclose candidate label and the PR stays open. Cosmetic changes or incomplete fixes will not be sufficient. Maintainers will assess improvements on their own schedule. Please do not ping (@) maintainers.

@thc1006

thc1006 commented Jul 28, 2026

Copy link
Copy Markdown
Author

This reads to me as completely AI-generated, and it's not clear if this or coercing _verts3d to an array is a better solution. Please write up your thoughts on this in your own voice without using AI, except for direct word-to-word translation.

Dear @scottshambaugh ,
OK I will rewrite content right now
Follow maybe other people PR or issue body or other best practice.

@scottshambaugh scottshambaugh removed the ai-contribution PRs that are AI generated without a human in the loop label Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: autoclose candidate PRs that are not yet ready for review and may be automatically closed in two weeks topic: mplot3d

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Line3D.draw requires .shape on _verts3d, but set_data_3d accepts and stores any array-like

3 participants