It does seem like most of the features are closer to collections, whether it be PolyCollection (LAND), LineCollection (COASTLINES). So this seems reasonable to me.

Are there any issues or benefits with exposing FeatureArtist.set_array() now if it is a collection? We currently have styler which takes in a geometry and styles that geometry. That seems like it could be deprecated and handled by the collection mapping through the geometries with this PR. So, possibly a nice potential of this approach by calling set_array() with values for each of the geometries within the feature similar to what geopandas does: https://geopandas.org/en/stable/docs/user_guide/mapping.html#choropleth-maps

Should the name be updated and called FeatureCollection, and then deprecate FeatureArtist and tell people to use the new names instead?

Thanks @greglucas!

I did spot #1789 and wondered if this would help there. I don't really know anything about ScalarMappables though. Does each element in the array correspond to a single path?

I think the point about styler is that when you are using something like Natural Earth the geometries get generated on the fly so the user doesn't necessarily know how many there are or what order they come in. They can also change if you pan or if you zoom with the auto-scaler. Even if you pass explicit geometries, a single geometry might be transformed to multiple geometries if its crs is different from the axes crs. styler gets applied after the transformation.

I also wondered about the name. Having Collection rather than Artist in the name would seem more correct in Matplotlib context. I'm not sure about FeatureCollection though as that seems to imply it handles multiple features (like a LineCollection draws multiple lines). Maybe GeometryCollection, GeomCollection or GeomSet (borrowing from ContourSet)? Within Cartopy having FeatureArtist vs Feature makes it fairly intuitive that the former has something to do with drawing even if you weren't familiar with Matplotlib. So maybe could go a different direction and have something like FeatureDrawer or ...? Hmmm, naming things is hard!

I don't really know anything about ScalarMappables though. Does each element in the array correspond to a single path?

Yes, they all get passed to the renderer at once within collections rather than one-by-one as Python objects.
https://github.com/matplotlib/matplotlib/blob/d2cc4d0b0a2e1e9e8a5c1f311f0e10ef8acdb9ef/lib/matplotlib/collections.py#L423-L429

That is a good point about not necessarily knowing the geometries and what is used. Do we actually have all the geometries, it is just not drawing the ones that are out of the view? I'm wondering if we could do some fancy thing with only sending the specific paths we're interested in to the renderer, even if the others still exist. That definitely seems like a lot more work and effort to keep track of all the potential split paths and whats in/out of view!

Naming things is hard :)
I'd say we should avoid Geometry to avoid conflicts with shapely's names: https://shapely.readthedocs.io/en/latest/reference/shapely.GeometryCollection.html I think your point about FeatureArtist being obvious is good, so it is fine to keep the name as it is.

All this to say, this seems like a good idea and enables some other interesting possibilities as follow-ups as well.

Hmmm...

import matplotlib.pyplot as plt
import cartopy.crs as ccrs
import shapely.geometry as sgeom

square1 = sgeom.Polygon([(-10, -10), (10, -10), (10, 10), (-10, 10), (-10, 10) ])
square2 = sgeom.Polygon([(30, 0), (50, 0), (50, 20), (30, 20), (30, 0)])

ax = plt.figure().add_subplot(111, projection=ccrs.PlateCarree())
ax.coastlines()

ax.add_geometries([square1, square2], crs=ccrs.PlateCarree(),
                  facecolors=['r', 'b'])

plt.show()

If we pan/zoom so that the red square is out of the picture, the blue square turns red!

Also,

ax = plt.figure().add_subplot(111, projection=ccrs.PlateCarree(central_longitude=180))
ax.coastlines()

ax.add_geometries([square1, square2], crs=ccrs.PlateCarree(),
                  facecolors=['r', 'b'])

This is also true with v0.22, so I haven't actually broken anything here.

Edit: #2325 addresses the second of these.

Do we actually have all the geometries, it is just not drawing the ones that are out of the view?

Currently draw only accesses the geometries that intersect with the map

If you call add_geometries then you are creating a ShapelyFeature instance which is distinct from Natural Earth, etc. So it might be reasonable to get hold of all the geometries specifically for the ShapelyFeature but leave as is for the rest?

If you call add_geometries then you are creating a ShapelyFeature instance which is distinct from Natural Earth, etc. So it might be reasonable to get hold of all the geometries specifically for the ShapelyFeature but leave as is for the rest?

I think this might be similar to what we are doing with the GeoQuadMesh, where a user has all the data and would call set_array() on all of their geometries, but then we dispatch out to whatever quads are specifically shown. So this draw method would just grab the facecolors and other attributes of the elements that it intends to draw.

I think I am pretty happy with this one now. I should squash the commits before merge but the history may be useful for review.

In particular, I found some weird behaviour when I made the change within draw at 5124784: four of the five image tests failed, as the blue square shifted position slightly. Only the test that uses styler was unaffected, but that one was still drawing a single path at a time. I made the styler test consistent with the others by adding an extra geometry so the styler test is now drawing two paths at once. I have no idea why drawing a single path vs multiple paths would make this difference and I'm also not sure whether the difference is big enough to matter - the RMS difference was ~3.7.

The doc failure appears to be because of WMTS shenanigans.

I think this now also closes #1789.