-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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.
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
BF: ignore CLOSEPOLY after NaN in PathNanRemover #17885
Conversation
I think we ignore the point entirely for p = Path(
[[0, 0], [np.nan, np.nan]],
[1, 79]) |
Great question. It leaves it as-is. (Same as previous behavior). >>> p.cleaned(remove_nans=True)
Path(array([[ 0., 0.],
[nan, nan],
[ 0., 0.]]), array([ 1, 79, 0], dtype=uint8)) I think this makes sense because
So this PR only changes the behavior when leaving a |
I'm aware that this doesn't play well with >>> p.cleaned(remove_nans=True, simplify=True)
Path(array([[ 0., 0.],
[nan, nan],
[nan, nan],
[ 0., 0.]]), array([1, 2, 2, 0], dtype=uint8)) but this is also existing behavior, and I haven't taken the time to grok the full |
Per the call, added tests that check the "optimized" code path (i.e. empty |
e90850f
to
b95ce02
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still approve.
b95ce02
to
e216e59
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed a small whitespace fix.
…885-on-v3.3.x Backport PR #17885 on branch v3.3.x (BF: ignore CLOSEPOLY after NaN in PathNanRemover)
PR Summary
Fixes #17868.
In short, the current
PathNanRemover
always keepsCLOSEPOLY
"vertices" around, even if they contain NaN. This is arguably okay (since it is documented in comments that the values ofCLOSEPOLY
vertices are ignored). However, because of this, if a CLOSEPOLY occurs before any non-NaN segments of a Path are encountered, thenPathNanRemover
(and, by proxy,Path.cleaned(remove_nans=True)
will return a malformedPath
that starts with aCLOSEPOLY
.Current behavior:
New behavior
PR Checklist