| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
I think we ignore the point entirely for CLOSEPOLY, but what happens for something like? p = Path(
[[0, 0], [np.nan, np.nan]],
[1, 79]) |
Sorry, something went wrong.
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 CLOSEPOLY in would result in an ill-defined Path. |
Sorry, something went wrong.
|
I'm aware that this doesn't play well with PathSimplifier: >>> 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 PathSimplifer algorithm yet to know whether or not it's easy to do even better. |
Sorry, something went wrong.
|
Per the call, added tests that check the "optimized" code path (i.e. empty codes array) and explicitly added the breaking example from #17868 as a test case. |
Sorry, something went wrong.
There was a problem hiding this comment.
Still approve.
Sorry, something went wrong.
There was a problem hiding this comment.
I pushed a small whitespace fix.
Sorry, something went wrong.
…885-on-v3.3.x Backport PR #17885 on branch v3.3.x (BF: ignore CLOSEPOLY after NaN in PathNanRemover)
| Back | FazBrowse Home | New Git URL |
PR Summary
Fixes #17868.
In short, the current PathNanRemover always keeps CLOSEPOLY "vertices" around, even if they contain NaN. This is arguably okay (since it is documented in comments that the values of CLOSEPOLY vertices are ignored). However, because of this, if a CLOSEPOLY occurs before any non-NaN segments of a Path are encountered, then PathNanRemover (and, by proxy, Path.cleaned(remove_nans=True) will return a malformed Path that starts with a CLOSEPOLY.
Current behavior:
New behavior
PR Checklist