[MNT]: Turn ContourSet into a (nearly) plain Collection #25128

anntzer · 2023-02-02T00:23:56Z

Summary

Currently, ContourSets are not Artists, which causes issues such as #6139 (essentially, the API is not uniform, and they do not appear directly in the draw tree).

At a low level, a ContourSet is represented as a list of PathCollections (the .collections attribute), with one such PathCollection per contour level value; the individual paths in the PathCollection are the connected components of that contour level. But we could instead "lift" things up one level and make ContourSet directly inherit from Collection (actually inheriting from PathCollection doesn't really help; despite the name, PathCollection is more designed for "resizable" paths such as scatter plots with variable sizes), and use a single Path object for each contour level (using MOVETOs as needed if the Path needs to be broken in multiple connected components. While slightly tedious, temporary backcompat with the old API could be provided via on-access creation of the "old" attributes, similarly to what was done in #24455.

There's actually (AFAICT) only one main difficulty with this approach, which is that draw_path_collection (called by Collection.draw) doesn't support hatching individual paths. However, this could be implemented by still implementing a ContourSet.draw which directly calls draw_path_collection if no hatching is needed, and in the other case re-decomposes the collection as needed to emit the right set of draw_path with the right hatching set.

attn @ianthomas23

Proposed fix

A first patch (which goes on top of #25121) is provided below.
Quite a few things (hatching, labeling, legends, etc.) are not implemented yet, but this at least allows one to call e.g. contour(np.random.rand(5, 5)) and get a contour plot.

diff --git a/doc/api/next_api_changes/deprecations/XXXXX-AL.rst b/doc/api/next_api_changes/deprecations/XXXXX-AL.rst
new file mode 100644
index 0000000000..caf6506209
--- /dev/null
+++ b/doc/api/next_api_changes/deprecations/XXXXX-AL.rst
@@ -0,0 +1,4 @@
+``QuadContourSet.allsegs`` and ``QuadContourSet.allkinds``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+These attributes are deprecated; directly retrieve the vertices and codes of
+the Path objects in ``QuadContourSet.collections`` if necessary.
diff --git a/lib/matplotlib/axes/_base.py b/lib/matplotlib/axes/_base.py
index db25af57ac..d46238bd97 100644
--- a/lib/matplotlib/axes/_base.py
+++ b/lib/matplotlib/axes/_base.py
@@ -2179,10 +2179,7 @@ class _AxesBase(martist.Artist):
         _api.check_isinstance(
             (mpl.contour.ContourSet, mcoll.Collection, mimage.AxesImage),
             im=im)
-        if isinstance(im, mpl.contour.ContourSet):
-            if im.collections[0] not in self._children:
-                raise ValueError("ContourSet must be in current Axes")
-        elif im not in self._children:
+        if im not in self._children:
             raise ValueError("Argument must be an image, collection, or "
                              "ContourSet in this Axes")
         self._current_image = im
diff --git a/lib/matplotlib/contour.py b/lib/matplotlib/contour.py
index 060d9990b0..f24b888cc0 100644
--- a/lib/matplotlib/contour.py
+++ b/lib/matplotlib/contour.py
@@ -3,7 +3,6 @@ Classes to support contour plotting and labelling for the Axes class.
 """
 
 import functools
-import itertools
 from numbers import Integral
 
 import numpy as np
@@ -474,6 +473,7 @@ class ContourLabeler:
 
         # calc_label_rot_and_inline() requires that (xmin, ymin)
         # be a vertex in the path. So, if it isn't, add a vertex here
+        # FIXME: Adapt to now API.
         paths = self.collections[conmin].get_paths()  # paths of correct coll.
         lc = paths[segmin].vertices  # vertices of correct segment
         # Where should the new vertex be added in data-units?
@@ -524,8 +524,9 @@ class ContourLabeler:
                 self.labelCValueList,
         )):
 
+            # FIXME: Adapt to new API.
             con = self.collections[icon]
-            trans = con.get_transform()
+            trans = self.get_transform()
             lw = self._get_nth_label_width(idx)
             additions = []
             paths = con.get_paths()
@@ -627,7 +628,7 @@ layers : array
 
 
 @_docstring.dedent_interpd
-class ContourSet(cm.ScalarMappable, ContourLabeler):
+class ContourSet(mcoll.Collection, ContourLabeler):
     """
     Store a set of contour lines or filled regions.
 
@@ -721,23 +722,28 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
             Keyword arguments are as described in the docstring of
             `~.Axes.contour`.
         """
+        if antialiased is None and filled:
+            # Eliminate artifacts; we are not stroking the boundaries.
+            antialiased = False
+            # The default for line contours will be taken from the
+            # LineCollection default, which uses :rc:`lines.antialiased`.
+        super().__init__(
+            antialiaseds=antialiased,
+            alpha=alpha,
+            transform=transform,
+        )
         self.axes = ax
         self.levels = levels
         self.filled = filled
-        self.linewidths = linewidths
-        self.linestyles = linestyles
+        # FIXME: We actually need a new draw() method which detects whether
+        # hatches is set (for filled contours) and, if so, performs the right
+        # decomposition to perform the draw as a series of draw_path (with
+        # hatching) instead of the single call to draw_path_collection.
         self.hatches = hatches
-        self.alpha = alpha
         self.origin = origin
         self.extent = extent
         self.colors = colors
         self.extend = extend
-        self.antialiased = antialiased
-        if self.antialiased is None and self.filled:
-            # Eliminate artifacts; we are not stroking the boundaries.
-            self.antialiased = False
-            # The default for line contours will be taken from the
-            # LineCollection default, which uses :rc:`lines.antialiased`.
 
         self.nchunk = nchunk
         self.locator = locator
@@ -758,8 +764,6 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
         if self.origin == 'image':
             self.origin = mpl.rcParams['image.origin']
 
-        self._transform = transform
-
         self.negative_linestyles = negative_linestyles
         # If negative_linestyles was not defined as a keyword argument, define
         # negative_linestyles with rcParams
@@ -803,85 +807,50 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
                 if self._extend_max:
                     cmap.set_over(self.colors[-1])
 
-        self.collections = cbook.silent_list(None)
-
         # label lists must be initialized here
         self.labelTexts = []
         self.labelCValues = []
 
-        kw = {'cmap': cmap}
+        self.set_cmap(cmap)
         if norm is not None:
-            kw['norm'] = norm
-        # sets self.cmap, norm if needed;
-        cm.ScalarMappable.__init__(self, **kw)
+            self.set_norm(norm)
         if vmin is not None:
             self.norm.vmin = vmin
         if vmax is not None:
             self.norm.vmax = vmax
         self._process_colors()
 
-        if getattr(self, 'allsegs', None) is None:
-            self.allsegs, self.allkinds = self._get_allsegs_and_allkinds()
-        elif self.allkinds is None:
-            # allsegs specified in constructor may or may not have allkinds as
-            # well.  Must ensure allkinds can be zipped below.
-            self.allkinds = [None] * len(self.allsegs)
-
-        # Each entry in (allsegs, allkinds) is a list of (segs, kinds) which
-        # specifies a list of Paths; but kinds can be None too in which case
-        # all paths in that list are codeless.
-        allpaths = [
-            [*map(mpath.Path,
-                  segs,
-                  kinds if kinds is not None else itertools.repeat(None))]
-            for segs, kinds in zip(self.allsegs, self.allkinds)]
+        if self._paths is None:
+            self._paths = self._make_paths_from_contour_generator()
 
         if self.filled:
-            if self.linewidths is not None:
+            if linewidths is not None:
                 _api.warn_external('linewidths is ignored by contourf')
             # Lower and upper contour levels.
             lowers, uppers = self._get_lowers_and_uppers()
             # Default zorder taken from Collection
             self._contour_zorder = kwargs.pop('zorder', 1)
+            self.set(
+                edgecolor="none",
+                zorder=self._contour_zorder,
+            )
 
-            self.collections[:] = [
-                mcoll.PathCollection(
-                    paths,
-                    antialiaseds=(self.antialiased,),
-                    edgecolors='none',
-                    alpha=self.alpha,
-                    transform=self.get_transform(),
-                    zorder=self._contour_zorder)
-                for level, level_upper, paths
-                in zip(lowers, uppers, allpaths)]
         else:
-            self.tlinewidths = tlinewidths = self._process_linewidths()
-            tlinestyles = self._process_linestyles()
-            aa = self.antialiased
-            if aa is not None:
-                aa = (self.antialiased,)
             # Default zorder taken from LineCollection, which is higher than
             # for filled contours so that lines are displayed on top.
             self._contour_zorder = kwargs.pop('zorder', 2)
+            self.set(
+                facecolor="none",
+                linewidths=self._process_linewidths(linewidths),
+                linestyle=self._process_linestyles(linestyles),
+                zorder=self._contour_zorder,
+                label="_nolegend_",
+            )
 
-            self.collections[:] = [
-                mcoll.PathCollection(
-                    paths,
-                    facecolors="none",
-                    antialiaseds=aa,
-                    linewidths=width,
-                    linestyles=[lstyle],
-                    alpha=self.alpha,
-                    transform=self.get_transform(),
-                    zorder=self._contour_zorder,
-                    label='_nolegend_')
-                for level, width, lstyle, paths
-                in zip(self.levels, tlinewidths, tlinestyles, allpaths)]
 
-        for col in self.collections:
-            self.axes.add_collection(col, autolim=False)
-            col.sticky_edges.x[:] = [self._mins[0], self._maxs[0]]
-            col.sticky_edges.y[:] = [self._mins[1], self._maxs[1]]
+        self.axes.add_collection(self, autolim=False)
+        self.sticky_edges.x[:] = [self._mins[0], self._maxs[0]]
+        self.sticky_edges.y[:] = [self._mins[1], self._maxs[1]]
         self.axes.update_datalim([self._mins, self._maxs])
         self.axes.autoscale_view(tight=True)
 
@@ -893,6 +862,15 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
                 ", ".join(map(repr, kwargs))
             )
 
+    allsegs = _api.deprecated("3.8")(property(lambda self: [
+        p.vertices for c in self.collections for p in c.get_paths()]))  # FIXME
+    allkinds = _api.deprecated("3.8")(property(lambda self: [
+        p.codes for c in self.collections for p in c.get_paths()]))  # FIXME
+
+    # FIXME: Provide backcompat aliases.
+    tlinewidths = ...
+    tcolors = ...
+
     def get_transform(self):
         """Return the `.Transform` instance used by this ContourSet."""
         if self._transform is None:
@@ -935,9 +913,10 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
         artists = []
         labels = []
 
+        # FIXME: Adapt to new API.
         if self.filled:
             lowers, uppers = self._get_lowers_and_uppers()
-            n_levels = len(self.collections)
+            n_levels = len(self._path_collection.get_paths())
 
             for i, (collection, lower, upper) in enumerate(
                     zip(self.collections, lowers, uppers)):
@@ -977,51 +956,63 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
         Must set self.levels, self.zmin and self.zmax, and update axes limits.
         """
         self.levels = args[0]
-        self.allsegs = args[1]
-        self.allkinds = args[2] if len(args) > 2 else None
+        allsegs = args[1]
+        allkinds = args[2] if len(args) > 2 else None
         self.zmax = np.max(self.levels)
         self.zmin = np.min(self.levels)
 
+        if allkinds is None:
+            allkinds = [[None] * len(segs) for segs in allsegs]
+
         # Check lengths of levels and allsegs.
         if self.filled:
-            if len(self.allsegs) != len(self.levels) - 1:
+            if len(allsegs) != len(self.levels) - 1:
                 raise ValueError('must be one less number of segments as '
                                  'levels')
         else:
-            if len(self.allsegs) != len(self.levels):
+            if len(allsegs) != len(self.levels):
                 raise ValueError('must be same number of segments as levels')
 
         # Check length of allkinds.
-        if (self.allkinds is not None and
-                len(self.allkinds) != len(self.allsegs)):
+        if len(allkinds) != len(allsegs):
             raise ValueError('allkinds has different length to allsegs')
 
         # Determine x, y bounds and update axes data limits.
-        flatseglist = [s for seg in self.allsegs for s in seg]
+        flatseglist = [s for seg in allsegs for s in seg]
         points = np.concatenate(flatseglist, axis=0)
         self._mins = points.min(axis=0)
         self._maxs = points.max(axis=0)
 
+        # Each entry in (allsegs, allkinds) is a list of (segs, kinds) which
+        # gets concatenated to construct a Path.
+        self._paths = [
+            mpath.Path.make_compound_path(*map(mpath.Path, segs, kinds))
+            for segs, kinds in zip(allsegs, allkinds)]
+
         return kwargs
 
-    def _get_allsegs_and_allkinds(self):
-        """Compute ``allsegs`` and ``allkinds`` using C extension."""
-        allsegs = []
-        allkinds = []
+    def _make_paths_from_contour_generator(self):
+        """Compute ``paths`` using C extension."""
+        if self._paths is not None:
+            return self._paths
+        paths = []
+        empty_path = mpath.Path(np.empty((0, 2)))
         if self.filled:
             lowers, uppers = self._get_lowers_and_uppers()
             for level, level_upper in zip(lowers, uppers):
                 vertices, kinds = \
                     self._contour_generator.create_filled_contour(
                         level, level_upper)
-                allsegs.append(vertices)
-                allkinds.append(kinds)
+                paths.append(
+                    mpath.Path(np.concatenate(vertices), np.concatenate(kinds))
+                    if len(vertices) else empty_path)
         else:
             for level in self.levels:
                 vertices, kinds = self._contour_generator.create_contour(level)
-                allsegs.append(vertices)
-                allkinds.append(kinds)
-        return allsegs, allkinds
+                paths.append(
+                    mpath.Path(np.concatenate(vertices), np.concatenate(kinds))
+                    if len(vertices) else empty_path)
+        return paths
 
     def _get_lowers_and_uppers(self):
         """
@@ -1048,18 +1039,11 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
         # so if vmin/vmax are not set yet, this would override them with
         # content from *cvalues* rather than levels like we want
         self.norm.autoscale_None(self.levels)
-        tcolors = [(tuple(rgba),)
-                   for rgba in self.to_rgba(self.cvalues, alpha=self.alpha)]
-        self.tcolors = tcolors
-        hatches = self.hatches * len(tcolors)
-        for color, hatch, collection in zip(tcolors, hatches,
-                                            self.collections):
-            if self.filled:
-                collection.set_facecolor(color)
-                # update the collection's hatch (may be None)
-                collection.set_hatch(hatch)
-            else:
-                collection.set_edgecolor(color)
+        tcolors = self.to_rgba(self.cvalues, alpha=self.alpha)
+        if self.filled:
+            self.set_facecolors(tcolors)
+        else:
+            self.set_edgecolors(tcolors)
         for label, cv in zip(self.labelTexts, self.labelCValues):
             label.set_alpha(self.alpha)
             label.set_color(self.labelMappable.to_rgba(cv))
@@ -1214,16 +1198,13 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
         if self.extend in ('both', 'max', 'min'):
             self.norm.clip = False
 
-        # self.tcolors are set by the "changed" method
-
-    def _process_linewidths(self):
-        linewidths = self.linewidths
+    def _process_linewidths(self, linewidths):
         Nlev = len(self.levels)
         if linewidths is None:
             default_linewidth = mpl.rcParams['contour.linewidth']
             if default_linewidth is None:
                 default_linewidth = mpl.rcParams['lines.linewidth']
-            tlinewidths = [(default_linewidth,)] * Nlev
+            tlinewidths = [default_linewidth] * Nlev
         else:
             if not np.iterable(linewidths):
                 linewidths = [linewidths] * Nlev
@@ -1234,11 +1215,10 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
                     linewidths = linewidths * nreps
                 if len(linewidths) > Nlev:
                     linewidths = linewidths[:Nlev]
-            tlinewidths = [(w,) for w in linewidths]
+            tlinewidths = [w for w in linewidths]
         return tlinewidths
 
-    def _process_linestyles(self):
-        linestyles = self.linestyles
+    def _process_linestyles(self, linestyles):
         Nlev = len(self.levels)
         if linestyles is None:
             tlinestyles = ['solid'] * Nlev
@@ -1319,7 +1299,7 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
             raise ValueError("Method does not support filled contours.")
 
         if indices is None:
-            indices = range(len(self.collections))
+            indices = range(len(self._paths))
 
         d2min = np.inf
         conmin = None
@@ -1330,6 +1310,7 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
 
         point = np.array([x, y])
 
+        # FIXME: Adapt to new API.
         for icon in indices:
             con = self.collections[icon]
             trans = con.get_transform()
@@ -1352,11 +1333,6 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
 
         return (conmin, segmin, imin, xmin, ymin, d2min)
 
-    def remove(self):
-        super().remove()
-        for coll in self.collections:
-            coll.remove()
-
 
 @_docstring.dedent_interpd
 class QuadContourSet(ContourSet):

The text was updated successfully, but these errors were encountered:

story645 · 2023-02-02T03:28:41Z

I'm assuming this would be an alternative to #24388 (given the overlap between contour and streamplot sets), which stalled out on collections not having remove methods and my not having the time/energy to sort that out, and this seems the better approach to me?

anntzer · 2023-02-02T08:33:18Z

It's not the same as the idea here is to use Collection and not Container (which are very different things -- one is in the draw tree and the other one is not); and ContourSets can already be removed just fine (currently by removing the individual component collections).

story645 · 2023-02-02T15:00:52Z

It's not the same as the idea here is to use Collection and not Container (which are very different things --

Well so that would be my question on that one too-to move the return type to a collection instead of to a container b/c they seem structured very similarly to me.

anntzer · 2023-02-02T15:07:30Z

They are not the same at all. A Container just holds a few artists together, which may be completely different from one another (in the case of StreamplotSet, a LineCollection (lines) and a PatchCollection (arrows) which exist as attributes) [sure, StreamplotSet is not a Container but it could be], whereas a Collection is for homogeneous "artist-like" objects (e.g. LineCollection is like a list of Line2Ds), which don't even have an independent existence (there isn't actually a bunch of Line2Ds as everything is directly in numpy arrays for efficiency).

story645 · 2023-02-02T15:14:42Z

Sorry wasn't trying to say that a collection was a container, I legit just forgot that streamplot was LineCollection + PatchCollection and thought it was a list of one artist type

Sorry for derailing the discussion here and thanks for clarifying

tacaswell · 2023-02-02T16:41:58Z

I am 👍🏻 in principle (but did not read the diff carefully). This is good both from a consistency point of view and for making a move to the data-prototype style artists easier.

ianthomas23 · 2023-02-03T09:50:20Z

Sounds very sensible to me.

Flattening each contour level into a single Path has consequences for filled contours. Currently each Path returned by contourpy for a contourf call represents a single outer boundary polygon and the zero or more hole polygons that it contains, so it intrinsically includes the relationship between outer boundaries and holes. If we flatten this to a single Path we discard the outer-to-hole relationship. This isn't a problem for rendering as all backends can deal with this, they essentially recalculated the outer-to-hole relationship as part of their rendering. tricontourf has always worked this way as it doesn't bother to calculate the outer-to-hole relationship.

There are a couple of ramifications of this:

Rendering will be slower as it usually scaled by O(n log(n)) where n is the number of points. I don't have benchmark times for this scenario though. My instinct is that it won't be much slower, but benchmark instincts are often wrong!
If we want a shim to support returning the original calculated segs and kinds, we can't just split the one big Path into individual polygons, we'd need to split it at the outer boundaries so would have to, for example, store an array of integer offsets to indicate where to split the big Path.

Looking ahead, there is future work relevant here. If we are not using the outer-to-hole relationship for rendering then we don't need to ask contourpy to calculate it. The new serial contouring algorithm supports FillType.ChunkCombinedCode which doesn't calculate the outer-to-hole relationship and returns, for each chunk, a single array of vertices and a single array of kind codes. This will make the code changes here simpler, and also give significant performance increases, e.g. combined calculation and agg rendering for contourpy's "random" benchmark is ~3 times faster here. Primarily this is because it avoids creating a large number of very small numpy arrays, but it also removes one level of looping in the Python code within mpl and avoids the outer-to-hole relationship calculations.

anntzer · 2023-02-17T23:34:42Z

Let's move the discussion to #25247.

QuLogic · 2023-06-30T05:00:58Z

Fixed by #25247.

anntzer added Maintenance topic: contour labels Feb 2, 2023

anntzer mentioned this issue Feb 2, 2023

Deprecate QuadContourSet.allsegs, .allkinds, .tcolors, .tlinewidths. #25138

Merged

6 tasks

anntzer mentioned this issue Feb 17, 2023

Turn ContourSet into a standard Collection artist. #25247

Merged

6 tasks

QuLogic closed this as completed Jun 30, 2023

QuLogic added this to the v3.8.0 milestone Jul 5, 2023

wpbonelli mentioned this issue Sep 19, 2023

bug: export_contours broken with matplotlib 3.8 modflowpy/flopy#1950

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MNT]: Turn ContourSet into a (nearly) plain Collection #25128

[MNT]: Turn ContourSet into a (nearly) plain Collection #25128

anntzer commented Feb 2, 2023

story645 commented Feb 2, 2023 •

edited

Loading

anntzer commented Feb 2, 2023

story645 commented Feb 2, 2023

anntzer commented Feb 2, 2023

story645 commented Feb 2, 2023

tacaswell commented Feb 2, 2023

ianthomas23 commented Feb 3, 2023

anntzer commented Feb 17, 2023

QuLogic commented Jun 30, 2023

[MNT]: Turn ContourSet into a (nearly) plain Collection #25128

[MNT]: Turn ContourSet into a (nearly) plain Collection #25128

Comments

anntzer commented Feb 2, 2023

Summary

Proposed fix

story645 commented Feb 2, 2023 • edited Loading

anntzer commented Feb 2, 2023

story645 commented Feb 2, 2023

anntzer commented Feb 2, 2023

story645 commented Feb 2, 2023

tacaswell commented Feb 2, 2023

ianthomas23 commented Feb 3, 2023

anntzer commented Feb 17, 2023

QuLogic commented Jun 30, 2023

story645 commented Feb 2, 2023 •

edited

Loading