Turn ContourSet into a standard Collection artist. #25247

anntzer · 2023-02-17T23:31:50Z

PR Summary

Closes (implements) #25128. Goes on top of #25138.

Keep (some) backcompat by making access to ContourSet.collection trigger
the self-replacement of the ContourSet by the old-style list of
PathCollections.

The baseline images are slighly shifted, but the new images actually
look more correct; see e.g. contour_corner_mask_False.png where the old
("-expected.png") implementation would white out some extra L-shaped
areas between masked regions (particularly visible in the diff image).

old:

new:

diff:

I suspect that many of the other diffs are due to tiny differences in rendering between drawing a single Path object with multiple blocks separated by MOVETO, and multiple separate Path objects (#25263 is one possible cause, though likely not the only one). I've checked all the diffs where I bumped the tol and at least they look reasonable, even though it may be possible to further reduce them by careful investigation. OTOH we can also accept the diffs and just regen the baselines.

PR Checklist

Documentation and Tests

Has pytest style unit tests (and pytest passes)
Documentation is sphinx and numpydoc compliant (the docs should build without error).
New plotting related features are documented with examples.

Release Notes

New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

greglucas

This is nice! I like that this can be a single collection without having to remember to go into the ContourSet.collections attribute. I tested all of the contour examples which all produced what I would expect with zoom/pan etc as well.

I can't say I followed everything in this, but logically it made sense and your comments were helpful along the way. I think a lot of the complexity is due to the complexity of the current ContourSet class itself and not really that you added a lot of extra complexity with this PR.

doc/api/next_api_changes/deprecations/25138-AL.rst

greglucas · 2023-02-19T22:02:37Z

lib/matplotlib/axes/_base.py

            raise ValueError("Argument must be an image, collection, or "
                             "ContourSet in this Axes")


No need to call out ContourSet anymore since it is a Collection.

Suggested change

raise ValueError("Argument must be an image, collection, or "

"ContourSet in this Axes")

raise ValueError("Argument must be an image or collection "

"in this Axes")

greglucas · 2023-03-15T23:25:22Z

galleries/examples/images_contours_and_fields/contour_demo.py

+lws = np.broadcast_to(CS.get_linewidth(), len(levels)).copy()
+lws[6] = 4
+CS.set_linewidth(lws)


This is a case where having a single collection is not quite as nice. I'm not sure how much this is done in practice though. Possibly users wanting to set the dashed negative linestyles and solid positive linestyles manually themselves?

Line 88 simplification:
lws = np.resize(CS.get_linewidth(), len(levels))

Indeed, good catch.

Or even lws = np.ones(len(levels)) * 2? Its not like the linewidths are a mystery to the user.

greglucas · 2023-03-15T23:25:58Z

galleries/examples/images_contours_and_fields/contour_image.py

-
-for c in cset2.collections:
-    c.set_linestyle('solid')
+cset2.set_linestyle('solid')


And here this is much nicer :)

greglucas · 2023-03-15T23:31:28Z

lib/matplotlib/tests/test_contour.py

@@ -14,6 +14,17 @@
 import pytest


+def _maybe_split_collections(do_split):


Add a comment here noting that this is a helper to test the 3.8 deprecations and that we should remove it after the deprecation lapses.

greglucas · 2023-03-16T01:36:05Z

lib/matplotlib/contour.py

@@ -1240,11 +1309,10 @@ def _process_linewidths(self):
                    linewidths = linewidths * nreps
                if len(linewidths) > Nlev:
                    linewidths = linewidths[:Nlev]
-            tlinewidths = [(w,) for w in linewidths]
+            tlinewidths = [w for w in linewidths]


I think you're guaranteed a list of linewidths from both paths above?

Suggested change

tlinewidths = [w for w in linewidths]

tlinewidths = linewidths

anntzer · 2023-03-17T07:59:38Z

Thanks, handled all comments.

jklymak · 2023-03-28T23:49:26Z

@ianthomas23 did you have a chance to look at this? Thanks!

ianthomas23 · 2023-03-29T09:00:26Z

@ianthomas23 did you have a chance to look at this? Thanks!

Not yet, but I will do. Thanks for the reminder.

ianthomas23

Looks good to me.

Noting that with the change of behaviour of .collections no longer returning information about the relationship between filled polygons and their holes, we will find out who has been using mpl to obtain such information rather than just render filled contours.

WeatherGod · 2023-03-30T13:37:43Z

Looking through this now. I need to double-check a couple of things in some old code of mine. I am a little concerned about setting some of these tolerances as high as they are. I'm leaning towards regenerating the test images. I tend to treat the tolerance parameter as a way to account for differences in things out of our control like compilers and machine architecture. Meanwhile, this is a change that is completely within our control and we need to judge if the change is desirable.

…

On Thu, Mar 30, 2023 at 5:27 AM Ian Thomas ***@***.***> wrote: ***@***.**** approved this pull request. Looks good to me. Noting that with the change of behaviour of .collections no longer returning information about the relationship between filled polygons and their holes, we will find out who has been using mpl to obtain such information rather than just render filled contours. — Reply to this email directly, view it on GitHub <#25247 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACHF6GSOXWCEER2SU33XJDW6VGXTANCNFSM6AAAAAAU77GGSA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

greglucas · 2023-03-30T14:05:42Z

It might be worth checking with downstream libraries as well? I just ran this PR with Cartopy and there is one failure with contour labeling, which I think should be addressable on the Cartopy-side since we override that method with some path manipulations.
Cartopy: https://github.com/SciTools/cartopy/blob/main/lib/cartopy/mpl/contour.py

Possibly check with Astropy? (looking at this code, this update may actually help them to only have one collection at a time going through their transform stack and avoid this utility function altogether): https://github.com/astropy/astropy/blob/8010bcb486087b1d3073a4f69f300f75d784d3e1/astropy/visualization/wcsaxes/utils.py#L118

anntzer · 2023-03-30T15:30:23Z

I'll try to go through the baseline images changes again.
@dstansby I think(?) you're involved with astropy; can you check whether this PR will cause problems for them?

tacaswell · 2023-06-28T19:31:56Z

I am going to merge this as-is and open a new issue to expand the API change notes on this to more thoroughly explain how to update the styling on a specific contour in the set.

rcomer · 2023-07-07T13:29:48Z

This has caused an image comparison failure in Cartopy, where the inline=True option for ax.clabel is no longer working. See SciTools/cartopy#2207.

jklymak · 2023-07-07T14:25:51Z

Can you make an issue with just Matplotlib? Seems crazy we have no tests of inline clabels.

rcomer · 2023-07-07T14:31:27Z

I tried running the Contour Label Demo in the same environment, and that looks fine. So I'm guessing the problem has something to do with transforms, which I've never done anything with directly. So not sure where to start with making a mpl-only example.

jklymak · 2023-07-07T15:51:31Z

Does it barf on log-log axes?

rcomer · 2023-07-07T17:19:37Z

Ah, I just realised Cartopy has its own clabel. I missed that before because I only looked on GeoAxes.
https://github.com/SciTools/cartopy/blob/c4858e4963fd8a11fe227700a4b97043f69986ab/lib/cartopy/mpl/contour.py#L22

Edit: and now I also see that @greglucas discussed this above 🤦‍♀️

mstoelinga · 2023-10-29T03:10:35Z

doc/api/next_api_changes/deprecations/25247-AL.rst

Unfortunately this change broke my "contours to shapefile" code, because the temporarily retained functionality of CountourSet.collections actually does not behave as it did before matplotlib 3.8.0. Somehow it is causing my final polygon set to become a single polygon, so that every contour has a connecting segment to the next contour in the set. So I was forced to update my code to the new definition of CountourSet, which I guess isn't a terrible thing, but effectively there is no "deprecation period"...it was immediate for me.

@mstoelinga, please try our main branch and see if this is fixed for you. This was a mistake, and should be fixed in 3.8.1

I think the collections attribute actually couldn’t be fully backwards compatible (from this comment)

matplotlib/lib/matplotlib/contour.py

Lines 967 to 970 in 518b268

# On access, make oneself invisible and instead add the old-style collections

# (one PathCollection per level). We do not try to further split contours into

# connected components as we already lost track of what pairs of contours need

# to be considered as single units to draw filled regions with holes.

There is now only one path in each collection, whereas there used to be multiple.

"contours to shapefile" code

This sounds like something that could use contourpy directly.

Very much so. I need to write up some docs on how to convert ContourPy outputs into other formats such as Shapely.

I am also looking for a solution to convert the ContourSet to shapefile. The code below does not work anymore.

from shapely.geometry import LineString contour = ax.conout(...) levels = contour.levels shps = [] lvls = [] for l, c in zip(levels, contour): for ps in c.get_paths(): shps.append(LineString(ps.vertices)) lvls.append(l) shp = gpd.GeoDataFrame({'level':lvls}, geometry=shps, )

Any quick fix? Thanks!

@ougx does this help?
https://contourpy.readthedocs.io/en/latest/user_guide/external/shapely.html

@rcomer thanks. Are the plotted contours identical to what ContourPy generates?

@ougx Matplotlib uses ContourPy's contours.

anntzer added API: consistency API: changes status: waiting for other PR topic: contour labels Feb 17, 2023

anntzer added this to the v3.8.0 milestone Feb 17, 2023

anntzer force-pushed the ca branch from b7623b2 to a275173 Compare February 17, 2023 23:32

This was referenced Feb 17, 2023

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

Closed

Support make_compound_path concatenating only empty paths. #25252

Merged

anntzer force-pushed the ca branch from a275173 to edf8605 Compare February 18, 2023 23:23

anntzer mentioned this pull request Feb 19, 2023

[Bug]: path snapping disabled for fully horizontal/vertical paths if unconnected path component is not fully vertical/horizontal #25263

Open

anntzer force-pushed the ca branch 6 times, most recently from 895a60e to 668aa49 Compare February 20, 2023 22:26

github-actions bot added the status: needs rebase label Feb 24, 2023

anntzer force-pushed the ca branch from 668aa49 to cdc9ff9 Compare March 10, 2023 09:28

github-actions bot removed the status: needs rebase label Mar 10, 2023

anntzer removed the status: waiting for other PR label Mar 10, 2023

greglucas approved these changes Mar 16, 2023

View reviewed changes

anntzer force-pushed the ca branch from cdc9ff9 to bac8350 Compare March 17, 2023 07:59

jklymak requested a review from ianthomas23 March 28, 2023 23:49

ianthomas23 approved these changes Mar 30, 2023

View reviewed changes

tacaswell merged commit f0f1339 into matplotlib:main Jun 28, 2023

tacaswell mentioned this pull request Jun 28, 2023

[Doc]: follow on to #25247 #26206

Closed

QuLogic mentioned this pull request Jun 30, 2023

'QuadContourSet' object has no attribute 'set_visible' or 'set_animated' #6139

Closed

pllim mentioned this pull request Jun 30, 2023

wcsaxes tests fail with "The collections attribute was deprecated in Matplotlib 3.8" astropy/astropy#15003

Closed

anntzer deleted the ca branch July 4, 2023 18:51

rcomer mentioned this pull request Jul 7, 2023

Contour labelling image test failing against Matplotlib main SciTools/cartopy#2207

Closed

This was referenced Jul 9, 2023

ENH: clip_path keyword for contour and contourf #26278

Merged

Use transformed paths for contour labelling decisions #26297

Closed

MNT: update contour for mpl v3.8 SciTools/cartopy#2213

Merged

anntzer mentioned this pull request Jul 15, 2023

[ENH]: scalermappable bar #26312

Open

This was referenced Jul 27, 2023

[Doc]: advice to use QuadContourSet.collections #26400

Closed

Avoid using deprecated ContourSet attributes SciTools/iris#5405

Merged

rcomer mentioned this pull request Aug 5, 2023

FIX: contourf nested contours SciTools/cartopy#2225

Merged

ksunden mentioned this pull request Aug 15, 2023

[Bug]: ValueError thrown when levels is set to a lower value than vmin when using contours method of Axes #26531

Closed

ksunden mentioned this pull request Sep 15, 2023

Update contourf call check for mpl 3.8 pydata/xarray#8186

Merged

wpbonelli mentioned this pull request Sep 19, 2023

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

Closed

zmoon mentioned this pull request Sep 21, 2023

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

Closed

kafitzgerald mentioned this pull request Sep 29, 2023

Remove matplotlib 3.8+ pin NCAR/geocat-viz#160

Closed

ksunden mentioned this pull request Oct 2, 2023

[Bug]: plt.clabel raises exception at very low DPI: ValueError: 'codes' must be a 1D list or array with the same length of 'vertices'. Your vertices have shape (2, 2) but your codes have shape (1,) #26971

Closed

jklymak mentioned this pull request Oct 12, 2023

[Bug]: find_nearest_contour deprecated with no replacement? #27070

Closed

anntzer mentioned this pull request Oct 12, 2023

[Bug]: Contour plot is not updating after customization made to the color bar in the generated plot. #23080

Open

mstoelinga reviewed Oct 29, 2023

View reviewed changes

anntzer mentioned this pull request Jun 19, 2024

add 'auto' state for boxplot's patch_artist #28420

Draft

5 tasks

timhoffm mentioned this pull request Oct 18, 2024

[Bug]: no longer able to set multiple hatch colors #28990

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Turn ContourSet into a standard Collection artist. #25247

Turn ContourSet into a standard Collection artist. #25247

anntzer commented Feb 17, 2023 •

edited

Loading

greglucas left a comment

greglucas Feb 19, 2023

greglucas Mar 15, 2023

efiring Jun 15, 2023

anntzer Jun 15, 2023

jklymak Jun 15, 2023

greglucas Mar 15, 2023

greglucas Mar 15, 2023

greglucas Mar 16, 2023

anntzer commented Mar 17, 2023

jklymak commented Mar 28, 2023

ianthomas23 commented Mar 29, 2023

ianthomas23 left a comment

WeatherGod commented Mar 30, 2023 via email

greglucas commented Mar 30, 2023

anntzer commented Mar 30, 2023

tacaswell commented Jun 28, 2023

rcomer commented Jul 7, 2023

jklymak commented Jul 7, 2023

rcomer commented Jul 7, 2023

jklymak commented Jul 7, 2023

rcomer commented Jul 7, 2023 •

edited

Loading

mstoelinga Oct 29, 2023

jklymak Oct 29, 2023

rcomer Oct 29, 2023 •

edited

Loading

QuLogic Oct 30, 2023

ianthomas23 Oct 30, 2023

ougx Dec 15, 2023

rcomer Dec 15, 2023

ougx Dec 15, 2023

rcomer Dec 16, 2023

		raise ValueError("Argument must be an image, collection, or "
		"ContourSet in this Axes")

		@@ -14,6 +14,17 @@
		import pytest


		def _maybe_split_collections(do_split):

	tlinewidths = [w for w in linewidths]
	tlinewidths = linewidths

	# On access, make oneself invisible and instead add the old-style collections
	# (one PathCollection per level). We do not try to further split contours into
	# connected components as we already lost track of what pairs of contours need
	# to be considered as single units to draw filled regions with holes.

Turn ContourSet into a standard Collection artist. #25247

Turn ContourSet into a standard Collection artist. #25247

Conversation

anntzer commented Feb 17, 2023 • edited Loading

PR Summary

PR Checklist

greglucas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer commented Mar 17, 2023

jklymak commented Mar 28, 2023

ianthomas23 commented Mar 29, 2023

ianthomas23 left a comment

Choose a reason for hiding this comment

WeatherGod commented Mar 30, 2023 via email

greglucas commented Mar 30, 2023

anntzer commented Mar 30, 2023

tacaswell commented Jun 28, 2023

rcomer commented Jul 7, 2023

jklymak commented Jul 7, 2023

rcomer commented Jul 7, 2023

jklymak commented Jul 7, 2023

rcomer commented Jul 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcomer Oct 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer commented Feb 17, 2023 •

edited

Loading

rcomer commented Jul 7, 2023 •

edited

Loading

rcomer Oct 29, 2023 •

edited

Loading