8000 get_clip_path checks for nan by leejjoon · Pull Request #11748 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

get_clip_path checks for nan #11748

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 24, 2020
Merged

get_clip_path checks for nan #11748

merged 2 commits into from
Sep 24, 2020

Conversation

leejjoon
Copy link
Contributor

PR Summary

Specifying an ill-defined clip_path leads to a seg-fault (#4448) with agg backends. Here is an minimal example.

import io
import numpy as np

import matplotlib.figure as mfigure
from matplotlib.projections import PolarAxes
import matplotlib.patches as mpatches
from matplotlib.backends.backend_agg import FigureCanvasAgg

fig = mfigure.Figure()
ax = fig.add_subplot(111)
l1, = ax.plot([-1, 1], [-1, 1])

tr = PolarAxes.PolarTransform()
p = mpatches.Polygon(np.arange(-3, 3).reshape((3, 2)), transform=tr)

l1.set_clip_path(p)

FigureCanvasAgg(fig).print_png(io.BytesIO())

The current PR simply fix this by making GraphicsContextBase.get_clip_path returns None if it contains values that are not finite.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer
Copy link
Contributor
anntzer commented Jul 23, 2018

I would rather prefer we detect the invalid path and error out appropriately...

anntzer
anntzer reviewed Mar 3, 2019
View reviewed changes
lib/matplotlib/backend_bases.py Outdated
if np.all(np.isfinite(tpath.vertices)):
return tpath, tr
else:
warnings.warn("Ill-defined clip_path detected. Returning None.")
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be _log.warning(...).

Copy link
Member

Choose a reason for hiding this comment

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

Which will also reduce the line length of that line by 1 and thus make Travis (flake8) pass.

@anntzer
Copy link
Contributor
anntzer commented Mar 3, 2019

Actually, with the warning I'm fine with this approach. Good to go modulo one comment.

jklymak
jklymak reviewed Sep 23, 2020
View reviewed changes
@jklymak jklymak force-pushed the get_clip_path_check_nan branch from f6ddd61 to 87a3413 Compare September 23, 2020 23:01
@jklymak
Copy link
Member
jklymak commented Sep 23, 2020

@leejjoon I made the change requested above and rebased on current master and pushed this to your branch. Hope that is OK

@jklymak jklymak added this to the v3.4.0 milestone Sep 23, 2020
@tacaswell tacaswell merged commit 94d73b7 into matplotlib:master Sep 24, 2020
@anntzer anntzer mentioned this pull request Feb 5, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anntzer anntzer anntzer left review comments

@timhoffm timhoffm timhoffm left review comments

@jklymak jklymak jklymak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.
6 participants
@leejjoon @anntzer @jklymak @timhoffm @tacaswell @QuLogic
0