Skip to content
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

[Doc]: ContourSet allsegs and allkinds after #25247 #26863

Closed
zmoon opened this issue Sep 21, 2023 · 9 comments · Fixed by #26908
Closed

[Doc]: ContourSet allsegs and allkinds after #25247 #26863

zmoon opened this issue Sep 21, 2023 · 9 comments · Fixed by #26908
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. topic: contour
Milestone

Comments

@zmoon
Copy link

zmoon commented Sep 21, 2023

Documentation Link

https://matplotlib.org/stable/api/contour_api.html

Problem

It seems that their descriptions are no longer accurate after #25247.

For example

allsegs : [level0segs, level1segs, ...]
List of all the polygon segments for all the *levels*.
For contour lines ``len(allsegs) == len(levels)``, and for
filled contour regions ``len(allsegs) = len(levels)-1``. The lists
should look like ::
level0segs = [polygon0, polygon1, ...]
polygon0 = [[x0, y0], [x1, y1], ...]

Now, level0segs (.allsegs[0]) is just path0 (single Nx2 array of vertices for all contours for level 0), not [polygon0, polygon1, ...].

Suggested improvement

No response

@jklymak
Copy link
Member

jklymak commented Sep 21, 2023

Yes this is a major change that needs better documentation if possible.

I ran into this just yesterday trying to make kmls for Google earth out of Matplotlib contours. I recall discussion about this being better done directly in contourpy now but was unsure how to proceed. It may be good for a doc update to describe how to do this in contourpy or link to the appropriate place in contourpy. @ianthomas23 @anntzer i think you were the proponents of this change?

@jklymak jklymak added this to the v3.8.1 milestone Sep 21, 2023
@jklymak
Copy link
Member

jklymak commented Sep 21, 2023

I think the init for ContourSet is probably still correct, just that the temporary shim for the properties has now changed the shape? This came in #25138

@jklymak
Copy link
Member

jklymak commented Sep 21, 2023

@zmoon, did you have a test case for this?

@anntzer
Copy link
Contributor

anntzer commented Sep 22, 2023

From a quick look (untested...) I would agree with @jklymak's comment, i.e. the init allsegs/allkinds is still a list of polygons/of polygon codes unlike the attribute (but note that even in the init you can pass all the polygons together as a single "polyline" including the relevant CLOSEPOLY & MOVETO codes in allkinds).

@ianthomas23
Copy link
Member

As for whether to use Matplotlib or ContourPy to calculate your contours, it depends on your use case. If you are rendering them in Matplotlib as well as doing something else such as transforming the coordinates or extracting some statistics about the lines/polygons, then use Matplotlib. Otherwise use ContourPy directly so that you don't have to concern yourself with Matplotlib Figures, Axes and path codes. You have choice in how the contour data is returned to you, and it will be both simpler and faster.

It may be good for a doc update to describe how to do this in contourpy or link to the appropriate place in contourpy

The Matplotlib contouring functions already link to ContourPy. There isn't an appropriate place in the ContourPy docs to link, it is all of the docs. They are short, the library only does one thing.

@jklymak
Copy link
Member

jklymak commented Sep 24, 2023

OK, the question is if we meant to make the contours harder to parse in Matplotlib? Because currently we no longer return a list of segments for each level in allsegs.

@rcomer
Copy link
Member

rcomer commented Sep 24, 2023

The allsegs and allkinds properties are deprecated and the user is encouraged to get the vertices and codes from the paths.
https://matplotlib.org/stable/api/prev_api_changes/api_changes_3.8.0.html#allsegs-allkinds-tcolors-and-tlinewidths-attributes-of-contourset

If we want to make it easier to get hold of individual segments we could make _iter_connected_components public?

@jklymak
Copy link
Member

jklymak commented Sep 24, 2023

Agreed that is the deprecation, but decoding paths is a lot harder than dealing with a list segments. Given that contourpy returns a list of segments I think it would be nice if we made that available to users. It seems that is where we were before the deprecation, so perhaps it should be reconsidered?

@jklymak
Copy link
Member

jklymak commented Sep 25, 2023

Finally got a chance to test this:

x, y = np.meshgrid(np.arange(0, 10, 2), np.arange(0, 10, 2))

z = np.sin(x) * np.cos(y)

plt.figure(figsize=(6, 2), dpi=200)
cs = plt.contour(x, y, z, levels=[0, 0.5])
print(cs.allsegs)

3.7.2:

[[array([[3.09152808, 0.        ],
       [4.        , 1.41228293],
       [6.        , 1.41228293],
       [6.44044969, 0.        ]]), array([[8.        , 1.41228293],
       [6.44044969, 2.        ],
       [6.44044969, 4.        ],
       [8.        , 4.81006071]]), array([[3.09152808, 8.        ],
       [2.        , 7.73681118],
       [0.        , 6.        ],
       [2.        , 4.81006071],
       [3.09152808, 4.        ],
       [3.09152808, 2.        ],
       [2.        , 1.41228293],
       [0.        , 0.        ]]), array([[8.        , 7.73681118],
       [6.44044969, 8.        ]]), array([[4.        , 4.81006071],
       [3.09152808, 6.        ],
       [4.        , 7.73681118],
       [6.        , 7.73681118],
       [6.44044969, 6.        ],
       [6.        , 4.81006071],
       [4.        , 4.81006071]])], [array([[2.49132399, 0.        ],
       [2.        , 0.63570373],
       [1.09975017, 0.        ]]), array([[8.        , 0.69854605],
       [7.22861227, 0.        ]]), array([[8.        , 6.82265422],
       [7.26130678, 6.        ],
       [8.        , 5.43637593]]), array([[2.        , 5.49152105],
       [2.46642636, 6.        ],
       [2.        , 6.74216553],
       [1.14536993, 6.        ],
       [2.        , 5.49152105]])]]

3.8.0:

MatplotlibDeprecationWarning: The collections attribute was deprecated in Matplotlib 3.8 and will be removed two minor releases later.
[array([[3.09152808, 0.        ],
       [4.        , 1.41228293],
       [6.        , 1.41228293],
       [6.44044969, 0.        ],
       [8.        , 1.41228293],
       [6.44044969, 2.        ],
       [6.44044969, 4.        ],
       [8.        , 4.81006071],
       [3.09152808, 8.        ],
       [2.        , 7.73681118],
       [0.        , 6.        ],
       [2.        , 4.81006071],
       [3.09152808, 4.        ],
       [3.09152808, 2.        ],
       [2.        , 1.41228293],
       [0.        , 0.        ],
       [8.        , 7.73681118],
       [6.44044969, 8.        ],
       [4.        , 4.81006071],
       [3.09152808, 6.        ],
       [4.        , 7.73681118],
       [6.        , 7.73681118],
       [6.44044969, 6.        ],
       [6.        , 4.81006071],
       [4.        , 4.81006071]]), array([[2.49132399, 0.        ],
       [2.        , 0.63570373],
       [1.09975017, 0.        ],
       [8.        , 0.69854605],
       [7.22861227, 0.        ],
       [8.        , 6.82265422],
       [7.26130678, 6.        ],
       [8.        , 5.43637593],
       [2.        , 5.49152105],
       [2.46642636, 6.        ],
       [2.        , 6.74216553],
       [1.14536993, 6.        ],
       [2.        , 5.49152105]])]

So clearly the new allsegs is very different from the old - I think this needs to be fixed for anyone who needed the old one, even if it is now deprecated.

Whether we keep the deprecation is a less urgent question, but if we do, then I think we need to provide someway of getting this information.

@jklymak jklymak added Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. and removed Documentation labels Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. topic: contour
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants