Repository: geopandas/geopandas

Issue:
BUG: plotting MultiPoints with values-based coloring not working correctly
Example from the tests:

```
from shapely.affinity import rotate
from shapely.geometry import Point, MultiPoint

N = 10
points = GeoSeries(Point(i, i) for i in range(N))
multipoint1 = MultiPoint(points)
multipoint2 = rotate(multipoint1, 90)
df = GeoDataFrame(
    {"geometry": [multipoint1, multipoint2], "values": [0, 1]}
)
```

If you plot this where the colors get determined based on values (so not `df.plot(color="red")`, but eg `df.plot(column="values")` or `df.plot(cmap="viridis")`), the colors are incorrect. In this case they are all the same, but that will depend on the number of geometries / number of points in the multipoints. 


The reason for this is that the `values` for the color get "expanded" twice:

https://github.com/geopandas/geopandas/blob/9fdeb1474185c8d2edaede428f57d258810d87b7/geopandas/plotting.py#L256-L258

and then those values are set as the "c" in kwargs, and expanded again:

https://github.com/geopandas/geopandas/blob/9fdeb1474185c8d2edaede428f57d258810d87b7/geopandas/plotting.py#L263-L274





Discussion:
martinfleis: Thanks a lot for a fix!

Isn't the situation the same for lines and polygons as well? Or this behaviour affects solely points? If former, it would be good to fix that all. It is a regression caused by #1385 after all.

Also, could you add a test for this?
jorisvandenbossche: I *think* it's only buggy for points, because it's specifically for points that we set the `values` as `kwargs["c"]`, and thus got "expanded" twice. But indeed, would be good to add a test for all cases.
gcaria: Thanks for the feedback !

It seems that tests for all geometric objects are already in place, but I've found something interesting.

The test  for `MultiPoint`s
https://github.com/geopandas/geopandas/blob/5856332704233fcd54696999d9ee4f16f1a1ebff/geopandas/tests/test_plotting.py#L295-L298

- only checks the first 2 of 20 plotted points;
- is missing the optional argument`lut=2` when calling `plt.get_cmap`.

The test for `MultiLineString`s seems alright at first glance
https://github.com/geopandas/geopandas/blob/59d3b2b7260b485e95e06a38f1c3e218b0a40a46/geopandas/tests/test_plotting.py#L525-L529

but I tried to run a standalone version of the test and found:

```python
N = 10
lines = GeoSeries([LineString([(0, i), (4, i + 0.5), (9, i)]) for i in range(N)],
                               index=list("ABCDEFGHIJ"),)

multiline1 = MultiLineString(lines.loc["A":"B"].values)
multiline2 = MultiLineString(lines.loc["C":"D"].values)
df = GeoDataFrame({"geometry": [multiline1, multiline2], "values": [0, 1]})

ax = df.plot(column="values")
ax.collections[0].get_facecolors()

# returns array([], shape=(0, 4), dtype=float64)
```
which means that the `zip` output used in `_check_colors` is empty.

I've then run a quick test (...of the tests) printing the `len`s of the `zip` inputs within `_check_colors` and found mismatches in these tests (sometimes the first input having `len=0` as in `test_multilinestrings`):

```
geopandas/tests/test_plotting.py::TestPointPlotting::test_colormap
geopandas/tests/test_plotting.py::TestLineStringPlotting::test_multilinestrings
geopandas/tests/test_plotting.py::TestPolygonPlotting::test_single_color
geopandas/tests/test_plotting.py::TestGeometryCollectionPlotting::test_colors
geopandas/tests/test_plotting.py::TestGeometryCollectionPlotting::test_values
geopandas/tests/test_plotting.py::TestPlotCollections::test_linestrings_values
geopandas/tests/test_plotting.py::TestPlotCollections::test_polygons
geopandas/tests/test_plotting.py::TestPlotCollections::test_polygons_values
```

Could you please verify I'm not hallucinating ?


martinfleis: @gcaria good catch! You are right, these tests seem to be buggy. As you may have already noticed, the plotting module would require a lot of care, there are bugs coming up relatively often and I don't even want to look into a list of issues here.

Do you know how to fix these? Would you be willing to look into that? Can be done in a separate PR (or this one, your choice).
gcaria: I don't know how to fix these issues off the top of my head, but I'm happy to take a look at them. Hopefully all these buggy tests have something in common which will make the fix easy.

I'll then fix `test_multipoints` for this PR and work on the rest in a new one.


martinfleis: Thanks @gcaria! (CI failure is not related)

I'll move our comment above to a new issue to keep a track of it.
jorisvandenbossche: > Could you please verify I'm not hallucinating ?

BTW, @gcaria, I also remember that when working on those tests in the past, I got different results when plotting something standalone in eg an IPython console versus running the same code as a test. For some reason, something in our setup triggers matplotlib to behave slightly different. But I never figured out what it was. 
Just to say that chances are you were not hallucinating .. ;) 

Patch:
diff --git a/geopandas/plotting.py b/geopandas/plotting.py
index ddf6d2e7..c1765695 100644
--- a/geopandas/plotting.py
+++ b/geopandas/plotting.py
@@ -281,8 +281,7 @@ def _plot_point_collection(
         raise ValueError("Can only specify one of 'values' and 'color' kwargs")
 
     geoms, multiindex = _flatten_multi_geoms(geoms)
-    if values is not None:
-        values = np.take(values, multiindex, axis=0)
+    # values are expanded below as kwargs["c"]
 
     x = [p.x for p in geoms]
     y = [p.y for p in geoms]


Test patch:
diff --git a/geopandas/tests/test_plotting.py b/geopandas/tests/test_plotting.py
index fe1eb27f..864ff2ad 100644
--- a/geopandas/tests/test_plotting.py
+++ b/geopandas/tests/test_plotting.py
@@ -314,13 +314,13 @@ class TestPointPlotting:
         _check_colors(4, ax.collections[0].get_facecolors(), [MPL_DFT_COLOR] * 4)
 
         ax = self.df2.plot(column="values")
-        cmap = plt.get_cmap()
+        cmap = plt.get_cmap(lut=2)
         expected_colors = [cmap(0)] * self.N + [cmap(1)] * self.N
-        _check_colors(2, ax.collections[0].get_facecolors(), expected_colors)
+        _check_colors(20, ax.collections[0].get_facecolors(), expected_colors)
 
         ax = self.df2.plot(color=["r", "b"])
         # colors are repeated for all components within a MultiPolygon
-        _check_colors(2, ax.collections[0].get_facecolors(), ["r"] * 10 + ["b"] * 10)
+        _check_colors(20, ax.collections[0].get_facecolors(), ["r"] * 10 + ["b"] * 10)
 
     def test_multipoints_alpha(self):
         ax = self.df2.plot(alpha=0.7)