-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
PathSimplifier
fails to ignore CLOSEPOLY
vertices
#17914
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
Comments
This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help! |
I think the bug is in the path simplification dealing with In [4]: p = Path([(0, 0), (1, 0), (1, 1), (np.nan, np.nan)],
...: [Path.MOVETO, Path.LINETO, Path.LINETO, Path.CLOSEPOLY])
In [9]: p.cleaned(simplify=True)
Out[9]:
Path(array([[ 0., 0.],
[ 1., 0.],
[ 1., 1.],
[nan, nan],
[nan, nan],
[ 0., 0.]]), array([1, 2, 2, 2, 2, 0], dtype=uint8))
In [13]: p.cleaned(simplify=True).cleaned(simplify=True)
Out[13]:
Path(array([[ 0., 0.],
[ 1., 0.],
[ 1., 1.],
[nan, nan],
[nan, nan],
[nan, nan],
[ 0., 0.]]), array([1, 2, 2, 2, 2, 2, 0], dtype=uint8))
In [14]: p.cleaned(simplify=True).cleaned(simplify=True)
Out[14]:
Path(array([[ 0., 0.],
[ 1., 0.],
[ 1., 1.],
[nan, nan],
[nan, nan],
[nan, nan],
[ 0., 0.]]), array([1, 2, 2, 2, 2, 2, 0], dtype=uint8))
In [15]: p.cleaned(simplify=True).cleaned(simplify=True).cleaned(simplify=True)
Out[15]:
Path(array([[ 0., 0.],
[ 1., 0.],
[ 1., 1.],
[nan, nan],
[nan, nan],
[nan, nan],
[nan, nan],
[ 0., 0.]]), array([1, 2, 2, 2, 2, 2, 2, 0], dtype=uint8)) at it seems to insert an extra LINETO nan everytime it is run! It is not surprising to me that the cleaning steps are not abealian so I am not sure I am too worried that In [17]: p.cleaned(simplify=True).cleaned(remove_nans=True)
Out[17]:
Path(array([[0., 0.],
[1., 0.],
[1., 1.],
[0., 0.]]), array([1, 2, 2, 0], dtype=uint8))
In [18]: p.cleaned(remove_nans=True).cleaned(simplify=True)
Out[18]:
Path(array([[ 0., 0.],
[ 1., 0.],
[ 1., 1.],
[nan, nan],
[nan, nan],
[ 0., 0.]]), array([1, 2, 2, 2, 2, 0], dtype=uint8)) are different and passing both keywords does the second: Lines 1033 to 1045 in f800bf6
The implemenation of the simplification is at matplotlib/src/path_converters.h Lines 694 to 912 in f800bf6
I suspect that the fix is to add a few lines of special-casing nan because nan is equal to nothing and all math involving nans results in nan so the "is the vector going the same direction enough" logic fails. I'm marking this as a good first issue (there is no API design and a pretty clear tast case), but has medium difficulty. To take this issue on you should know both Python and C++ pretty well (including templates, classes, and understanding the |
Good first issue - notes for new contributorsThis issue is suited to new contributors because it does not require understanding of the Matplotlib internals. To get started, please see our contributing guide. We do not assign issues. Check the Development section in the sidebar for linked pull requests (PRs). If there are none, feel free to start working on it. If there is an open PR, please collaborate on the work by reviewing it rather than duplicating it in a competing PR. If something is unclear, please reach out on any of our communication channels. |
Bug report
Bug summary
This bug does not seem to have surfaced anywhere yet, but
Path
's withNaN
vertices atCLOSEPOLY
can come throughPathNanRemover
in a "correct" state but thenPathSimplifier
can sometimes break it:Code for reproduction/Actual Outcome
Expected outcome
The values of the vertices in a
CLOSEPOLY
should always be ignored, in favor of the most recentMOVETO
's vertex values:Matplotlib version
print(matplotlib.get_backend())
):The text was updated successfully, but these errors were encountered: