Skip to content

BF: ignore CLOSEPOLY after NaN in PathNanRemover #17885

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

Merged
merged 2 commits into from
Jul 15, 2020
Merged

BF: ignore CLOSEPOLY after NaN in PathNanRemover #17885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11d00b5
BF: ignore CLOSEPOLY after NaN in PathNanRemover
brunobeltran Jul 11, 2020
e216e59
TESTS: ensure removing nan's doesn't break bar
brunobeltran Jul 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/matplotlib/tests/test_axes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1404,6 +1404,11 @@ def test_bar_tick_label_single():
ax.bar("a", "b", align='edge', tick_label='0', data=data)


def test_nan_bar_values():
fig, ax = plt.subplots()
ax.bar([0, 1], [np.nan, 4])


def test_bar_ticklabel_fail():
fig, ax = plt.subplots()
ax.bar([], [])
Expand Down
23 changes: 23 additions & 0 deletions lib/matplotlib/tests/test_path.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,3 +452,26 @@ def test_intersect_zero_length_segment():

assert outline_path.intersects_path(this_path)
assert this_path.intersects_path(outline_path)


def test_cleanup_closepoly():
# if the first connected component of a Path ends in a CLOSEPOLY, but that
# component contains a NaN, then Path.cleaned should ignore not just the
# control points but also the CLOSEPOLY, since it has nowhere valid to
# point.
paths = [
Path([[np.nan, np.nan], [np.nan, np.nan]],
[Path.MOVETO, Path.CLOSEPOLY]),
# we trigger a different path in the C++ code if we don't pass any
# codes explicitly, so we must also make sure that this works
Path([[np.nan, np.nan], [np.nan, np.nan]]),
# we should also make sure that this cleanup works if there's some
# multi-vertex curves
Path([[np.nan, np.nan], [np.nan, np.nan], [np.nan, np.nan],
[np.nan, np.nan]],
[Path.MOVETO, Path.CURVE3, Path.CURVE3, Path.CLOSEPOLY])
]
for p in paths:
cleaned = p.cleaned(remove_nans=True)
assert len(cleaned) == 1
assert cleaned.codes[0] == Path.STOP
21 changes: 16 additions & 5 deletions src/path_converters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ class PathNanRemover : protected EmbeddedQueue<4>
VertexSource *m_source;
bool m_remove_nans;
bool m_has_curves;
bool valid_segment_exists;

public:
/* has_curves should be true if the path contains bezier curve
Expand All @@ -172,7 +173,9 @@ class PathNanRemover : protected EmbeddedQueue<4>
PathNanRemover(VertexSource &source, bool remove_nans, bool has_curves)
: m_source(&source), m_remove_nans(remove_nans), m_has_curves(has_curves)
{
// empty
// ignore all close/end_poly commands until after the first valid
// (nan-free) command is encountered
valid_segment_exists = false;
}

inline void rewind(unsigned path_id)
Expand Down Expand Up @@ -202,8 +205,13 @@ class PathNanRemover : protected EmbeddedQueue<4>
are found along the way, the queue is emptied, and
the next curve segment is handled. */
code = m_source->vertex(x, y);
/* The vertices attached to STOP and CLOSEPOLY left are never
used, so we leave them as-is even if NaN. However, CLOSEPOLY
only makes sense if a valid MOVETO command has already been
emitted. */
if (code == agg::path_cmd_stop ||
code == (agg::path_cmd_end_poly | agg::path_flags_close)) {
(code == (agg::path_cmd_end_poly | agg::path_flags_close) &&
valid_segment_exists)) {
return code;
}

Expand All @@ -224,6 +232,7 @@ class PathNanRemover : protected EmbeddedQueue<4>
}

if (!has_nan) {
valid_segment_exists = true;
break;
}

Expand Down Expand Up @@ -251,21 +260,23 @@ class PathNanRemover : protected EmbeddedQueue<4>
code = m_source->vertex(x, y);

if (code == agg::path_cmd_stop ||
code == (agg::path_cmd_end_poly | agg::path_flags_close)) {
(code == (agg::path_cmd_end_poly | agg::path_flags_close) &&
valid_segment_exists)) {
return code;
}

if (!(std::isfinite(*x) && std::isfinite(*y))) {
do {
code = m_source->vertex(x, y);
if (code == agg::path_cmd_stop ||
code == (agg::path_cmd_end_poly | agg::path_flags_close)) {
(code == (agg::path_cmd_end_poly | agg::path_flags_close) &&
valid_segment_exists)) {
return code;
}
} while (!(std::isfinite(*x) && std::isfinite(*y)));
return agg::path_cmd_move_to;
}

valid_segment_exists = true;
return code;
}
}
Expand Down