Repository: geopandas/geopandas

Issue:
geopandas.plotting.plot_point_collection
geopandas.plotting.plot_point_collection(ax, shapely.geometry.Point(1,2))

x = geoms.x.values
y = geoms.y.values
, line 193, in plot_point_collection
    x = geoms.x.values
AttributeError: 'float' object has no attribute 'values'

geopandas.plotting.plot_point_collection(ax, [shapely.geometry.Point(1,2), shapely.geometry.Point(3,3)])

line 193, in plot_point_collection
    x = geoms.x.values

AttributeError: 'list' object has no attribute 'x'

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/geopandas/geopandas/pull/656?src=pr&el=h1) Report
> Merging [#656](https://codecov.io/gh/geopandas/geopandas/pull/656?src=pr&el=desc) into [master](https://codecov.io/gh/geopandas/geopandas/commit/36c8bcda34e4f46315eda812b11a0d9db32d01f2?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/geopandas/geopandas/pull/656/graphs/tree.svg?width=650&height=150&src=pr&token=6TOPMcCBhB)](https://codecov.io/gh/geopandas/geopandas/pull/656?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #656   +/-   ##
=======================================
  Coverage   94.83%   94.83%           
=======================================
  Files          14       14           
  Lines        1065     1065           
=======================================
  Hits         1010     1010           
  Misses         55       55
```


| [Impacted Files](https://codecov.io/gh/geopandas/geopandas/pull/656?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [geopandas/plotting.py](https://codecov.io/gh/geopandas/geopandas/pull/656/diff?src=pr&el=tree#diff-Z2VvcGFuZGFzL3Bsb3R0aW5nLnB5) | `93.93% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/geopandas/geopandas/pull/656?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/geopandas/geopandas/pull/656?src=pr&el=footer). Last update [36c8bcd...ed1caa5](https://codecov.io/gh/geopandas/geopandas/pull/656?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

jdmcbr: @martinfleis @jorisvandenbossche I got this at least to a merge-able state since I saw it came up recently. It seems like something along these lines would still be useful, though maybe a better way than what I did here. 
martinfleis: I fully support this PR. Keeping these plotting methods part of our public API was probably never intended and we do not approach them as such anyway. 
martinfleis: I have merged in changes from master and added test for DeprecationWarning. Once green we are ready to merge as there were no objections against and it is not even breaking change (for now).

Patch:
diff --git a/geopandas/plotting.py b/geopandas/plotting.py
index f915a569..6b0b096d 100644
--- a/geopandas/plotting.py
+++ b/geopandas/plotting.py
@@ -6,6 +6,21 @@ import pandas as pd
 import geopandas
 
 
+def deprecated(new):
+    """Helper to provide deprecation warning."""
+
+    def old(*args, **kwargs):
+        warnings.warn(
+            "{} is intended for internal ".format(new.__name__[1:])
+            + "use only, and will be deprecated.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        new(*args, **kwargs)
+
+    return old
+
+
 def _flatten_multi_geoms(geoms, prefix="Multi"):
     """
     Returns Series like geoms and index, except that any Multi geometries
@@ -17,7 +32,6 @@ def _flatten_multi_geoms(geoms, prefix="Multi"):
 
     Returns
     -------
-
     components : list of geometry
 
     component_index : index array
@@ -40,7 +54,7 @@ def _flatten_multi_geoms(geoms, prefix="Multi"):
     return components, np.array(component_index)
 
 
-def plot_polygon_collection(
+def _plot_polygon_collection(
     ax, geoms, values=None, color=None, cmap=None, vmin=None, vmax=None, **kwargs
 ):
     """
@@ -48,32 +62,25 @@ def plot_polygon_collection(
 
     Parameters
     ----------
-
     ax : matplotlib.axes.Axes
         where shapes will be plotted
-
     geoms : a sequence of `N` Polygons and/or MultiPolygons (can be mixed)
 
     values : a sequence of `N` values, optional
         Values will be mapped to colors using vmin/vmax/cmap. They should
         have 1:1 correspondence with the geometries (not their components).
         Otherwise follows `color` / `facecolor` kwargs.
-
     edgecolor : single color or sequence of `N` colors
         Color for the edge of the polygons
-
     facecolor : single color or sequence of `N` colors
         Color to fill the polygons. Cannot be used together with `values`.
-
     color : single color or sequence of `N` colors
         Sets both `edgecolor` and `facecolor`
-
     **kwargs
         Additional keyword arguments passed to the collection
 
     Returns
     -------
-
     collection : matplotlib.collections.Collection that was plotted
     """
 
@@ -130,7 +137,10 @@ def plot_polygon_collection(
     return collection
 
 
-def plot_linestring_collection(
+plot_polygon_collection = deprecated(_plot_polygon_collection)
+
+
+def _plot_linestring_collection(
     ax, geoms, values=None, color=None, cmap=None, vmin=None, vmax=None, **kwargs
 ):
     """
@@ -138,25 +148,19 @@ def plot_linestring_collection(
 
     Parameters
     ----------
-
     ax : matplotlib.axes.Axes
         where shapes will be plotted
-
     geoms : a sequence of `N` LineStrings and/or MultiLineStrings (can be
             mixed)
-
     values : a sequence of `N` values, optional
         Values will be mapped to colors using vmin/vmax/cmap. They should
         have 1:1 correspondence with the geometries (not their components).
-
     color : single color or sequence of `N` colors
         Cannot be used together with `values`.
 
     Returns
     -------
-
     collection : matplotlib.collections.Collection that was plotted
-
     """
     from matplotlib.collections import LineCollection
     from matplotlib.colors import is_color_like
@@ -194,7 +198,10 @@ def plot_linestring_collection(
     return collection
 
 
-def plot_point_collection(
+plot_linestring_collection = deprecated(_plot_linestring_collection)
+
+
+def _plot_point_collection(
     ax,
     geoms,
     values=None,
@@ -264,6 +271,9 @@ def plot_point_collection(
     return collection
 
 
+plot_point_collection = deprecated(_plot_point_collection)
+
+
 def plot_series(s, cmap=None, color=None, ax=None, figsize=None, **style_kwds):
     """
     Plot a GeoSeries.
@@ -369,7 +379,7 @@ def plot_series(s, cmap=None, color=None, ax=None, figsize=None, **style_kwds):
             facecolor = color
 
         values_ = values[poly_idx] if cmap else None
-        plot_polygon_collection(
+        _plot_polygon_collection(
             ax, polys, values_, facecolor=facecolor, cmap=cmap, **style_kwds
         )
 
@@ -377,7 +387,7 @@ def plot_series(s, cmap=None, color=None, ax=None, figsize=None, **style_kwds):
     lines = expl_series[line_idx]
     if not lines.empty:
         values_ = values[line_idx] if cmap else None
-        plot_linestring_collection(
+        _plot_linestring_collection(
             ax, lines, values_, color=color, cmap=cmap, **style_kwds
         )
 
@@ -385,7 +395,9 @@ def plot_series(s, cmap=None, color=None, ax=None, figsize=None, **style_kwds):
     points = expl_series[point_idx]
     if not points.empty:
         values_ = values[point_idx] if cmap else None
-        plot_point_collection(ax, points, values_, color=color, cmap=cmap, **style_kwds)
+        _plot_point_collection(
+            ax, points, values_, color=color, cmap=cmap, **style_kwds
+        )
 
     plt.draw()
     return ax
@@ -616,7 +628,7 @@ def plot_dataframe(
     polys = expl_series[poly_idx & np.invert(nan_idx)]
     subset = values[poly_idx & np.invert(nan_idx)]
     if not polys.empty:
-        plot_polygon_collection(
+        _plot_polygon_collection(
             ax, polys, subset, vmin=mn, vmax=mx, cmap=cmap, **style_kwds
         )
 
@@ -624,7 +636,7 @@ def plot_dataframe(
     lines = expl_series[line_idx & np.invert(nan_idx)]
     subset = values[line_idx & np.invert(nan_idx)]
     if not lines.empty:
-        plot_linestring_collection(
+        _plot_linestring_collection(
             ax, lines, subset, vmin=mn, vmax=mx, cmap=cmap, **style_kwds
         )
 
@@ -635,7 +647,7 @@ def plot_dataframe(
         if isinstance(markersize, np.ndarray):
             markersize = np.take(markersize, multiindex, axis=0)
             markersize = markersize[point_idx & np.invert(nan_idx)]
-        plot_point_collection(
+        _plot_point_collection(
             ax,
             points,
             subset,
@@ -746,7 +758,6 @@ def _mapclassify_choro(values, scheme, **classification_kwds):
     binning
         Binning objects that holds the Series with values replaced with
         class identifier and the bins.
-
     """
     try:
         import mapclassify.classifiers as classifiers


Test patch:
diff --git a/geopandas/tests/test_plotting.py b/geopandas/tests/test_plotting.py
index 306e3b6c..5ffacf1c 100644
--- a/geopandas/tests/test_plotting.py
+++ b/geopandas/tests/test_plotting.py
@@ -744,40 +744,40 @@ class TestPlotCollections:
         # failing with matplotlib 1.4.3 (edge stays black even when specified)
         pytest.importorskip("matplotlib", "1.5.0")
 
-        from geopandas.plotting import plot_point_collection
+        from geopandas.plotting import _plot_point_collection, plot_point_collection
         from matplotlib.collections import PathCollection
 
         fig, ax = plt.subplots()
-        coll = plot_point_collection(ax, self.points)
+        coll = _plot_point_collection(ax, self.points)
         assert isinstance(coll, PathCollection)
         ax.cla()
 
         # default: single default matplotlib color
-        coll = plot_point_collection(ax, self.points)
+        coll = _plot_point_collection(ax, self.points)
         _check_colors(self.N, coll.get_facecolors(), [MPL_DFT_COLOR] * self.N)
         # edgecolor depends on matplotlib version
         # _check_colors(self.N, coll.get_edgecolors(), [MPL_DFT_COLOR]*self.N)
         ax.cla()
 
         # specify single other color
-        coll = plot_point_collection(ax, self.points, color="g")
+        coll = _plot_point_collection(ax, self.points, color="g")
         _check_colors(self.N, coll.get_facecolors(), ["g"] * self.N)
         _check_colors(self.N, coll.get_edgecolors(), ["g"] * self.N)
         ax.cla()
 
         # specify edgecolor/facecolor
-        coll = plot_point_collection(ax, self.points, facecolor="g", edgecolor="r")
+        coll = _plot_point_collection(ax, self.points, facecolor="g", edgecolor="r")
         _check_colors(self.N, coll.get_facecolors(), ["g"] * self.N)
         _check_colors(self.N, coll.get_edgecolors(), ["r"] * self.N)
         ax.cla()
 
         # list of colors
-        coll = plot_point_collection(ax, self.points, color=["r", "g", "b"])
+        coll = _plot_point_collection(ax, self.points, color=["r", "g", "b"])
         _check_colors(self.N, coll.get_facecolors(), ["r", "g", "b"])
         _check_colors(self.N, coll.get_edgecolors(), ["r", "g", "b"])
         ax.cla()
 
-        coll = plot_point_collection(
+        coll = _plot_point_collection(
             ax,
             self.points,
             color=[(0.5, 0.5, 0.5, 0.5), (0.1, 0.2, 0.3, 0.5), (0.4, 0.5, 0.6, 0.5)],
@@ -796,14 +796,18 @@ class TestPlotCollections:
 
         # not a color
         with pytest.raises(TypeError):
-            plot_point_collection(ax, self.points, color="not color")
+            _plot_point_collection(ax, self.points, color="not color")
+
+        # check DeprecationWarning
+        with pytest.warns(DeprecationWarning):
+            plot_point_collection(ax, self.points)
 
     def test_points_values(self):
-        from geopandas.plotting import plot_point_collection
+        from geopandas.plotting import _plot_point_collection
 
         # default colormap
         fig, ax = plt.subplots()
-        coll = plot_point_collection(ax, self.points, self.values)
+        coll = _plot_point_collection(ax, self.points, self.values)
         fig.canvas.draw_idle()
         cmap = plt.get_cmap()
         expected_colors = cmap(np.arange(self.N) / (self.N - 1))
@@ -812,36 +816,39 @@ class TestPlotCollections:
         # _check_colors(self.N, coll.get_edgecolors(), expected_colors)
 
     def test_linestrings(self):
-        from geopandas.plotting import plot_linestring_collection
+        from geopandas.plotting import (
+            _plot_linestring_collection,
+            plot_linestring_collection,
+        )
         from matplotlib.collections import LineCollection
 
         fig, ax = plt.subplots()
-        coll = plot_linestring_collection(ax, self.lines)
+        coll = _plot_linestring_collection(ax, self.lines)
         assert isinstance(coll, LineCollection)
         ax.cla()
 
         # default: single default matplotlib color
-        coll = plot_linestring_collection(ax, self.lines)
+        coll = _plot_linestring_collection(ax, self.lines)
         _check_colors(self.N, coll.get_color(), [MPL_DFT_COLOR] * self.N)
         ax.cla()
 
         # specify single other color
-        coll = plot_linestring_collection(ax, self.lines, color="g")
+        coll = _plot_linestring_collection(ax, self.lines, color="g")
         _check_colors(self.N, coll.get_colors(), ["g"] * self.N)
         ax.cla()
 
         # specify edgecolor / facecolor
-        coll = plot_linestring_collection(ax, self.lines, facecolor="g", edgecolor="r")
+        coll = _plot_linestring_collection(ax, self.lines, facecolor="g", edgecolor="r")
         _check_colors(self.N, coll.get_facecolors(), ["g"] * self.N)
         _check_colors(self.N, coll.get_edgecolors(), ["r"] * self.N)
         ax.cla()
 
         # list of colors
-        coll = plot_linestring_collection(ax, self.lines, color=["r", "g", "b"])
+        coll = _plot_linestring_collection(ax, self.lines, color=["r", "g", "b"])
         _check_colors(self.N, coll.get_colors(), ["r", "g", "b"])
         ax.cla()
 
-        coll = plot_linestring_collection(
+        coll = _plot_linestring_collection(
             ax,
             self.lines,
             color=[(0.5, 0.5, 0.5, 0.5), (0.1, 0.2, 0.3, 0.5), (0.4, 0.5, 0.6, 0.5)],
@@ -854,7 +861,7 @@ class TestPlotCollections:
         ax.cla()
 
         # pass through of kwargs
-        coll = plot_linestring_collection(ax, self.lines, linestyle="--", linewidth=1)
+        coll = _plot_linestring_collection(ax, self.lines, linestyle="--", linewidth=1)
         exp_ls = _style_to_linestring_onoffseq("dashed", 1)
         res_ls = coll.get_linestyle()[0]
         assert res_ls[0] == exp_ls[0]
@@ -863,15 +870,19 @@ class TestPlotCollections:
 
         # not a color
         with pytest.raises(TypeError):
-            plot_linestring_collection(ax, self.lines, color="not color")
+            _plot_linestring_collection(ax, self.lines, color="not color")
+
+        # check DeprecationWarning
+        with pytest.warns(DeprecationWarning):
+            plot_linestring_collection(ax, self.lines)
 
     def test_linestrings_values(self):
-        from geopandas.plotting import plot_linestring_collection
+        from geopandas.plotting import _plot_linestring_collection
 
         fig, ax = plt.subplots()
 
         # default colormap
-        coll = plot_linestring_collection(ax, self.lines, self.values)
+        coll = _plot_linestring_collection(ax, self.lines, self.values)
         fig.canvas.draw_idle()
         cmap = plt.get_cmap()
         expected_colors = cmap(np.arange(self.N) / (self.N - 1))
@@ -879,7 +890,7 @@ class TestPlotCollections:
         ax.cla()
 
         # specify colormap
-        coll = plot_linestring_collection(ax, self.lines, self.values, cmap="RdBu")
+        coll = _plot_linestring_collection(ax, self.lines, self.values, cmap="RdBu")
         fig.canvas.draw_idle()
         cmap = plt.get_cmap("RdBu")
         expected_colors = cmap(np.arange(self.N) / (self.N - 1))
@@ -887,7 +898,7 @@ class TestPlotCollections:
         ax.cla()
 
         # specify vmin/vmax
-        coll = plot_linestring_collection(ax, self.lines, self.values, vmin=3, vmax=5)
+        coll = _plot_linestring_collection(ax, self.lines, self.values, vmin=3, vmax=5)
         fig.canvas.draw_idle()
         cmap = plt.get_cmap()
         expected_colors = cmap([0])
@@ -895,33 +906,33 @@ class TestPlotCollections:
         ax.cla()
 
     def test_polygons(self):
-        from geopandas.plotting import plot_polygon_collection
+        from geopandas.plotting import _plot_polygon_collection, plot_polygon_collection
         from matplotlib.collections import PatchCollection
 
         fig, ax = plt.subplots()
-        coll = plot_polygon_collection(ax, self.polygons)
+        coll = _plot_polygon_collection(ax, self.polygons)
         assert isinstance(coll, PatchCollection)
         ax.cla()
 
         # default: single default matplotlib color
-        coll = plot_polygon_collection(ax, self.polygons)
+        coll = _plot_polygon_collection(ax, self.polygons)
         _check_colors(self.N, coll.get_facecolor(), [MPL_DFT_COLOR] * self.N)
         _check_colors(self.N, coll.get_edgecolor(), ["k"] * self.N)
         ax.cla()
 
         # default: color sets both facecolor and edgecolor
-        coll = plot_polygon_collection(ax, self.polygons, color="g")
+        coll = _plot_polygon_collection(ax, self.polygons, color="g")
         _check_colors(self.N, coll.get_facecolor(), ["g"] * self.N)
         _check_colors(self.N, coll.get_edgecolor(), ["g"] * self.N)
         ax.cla()
 
         # default: color can be passed as a list
-        coll = plot_polygon_collection(ax, self.polygons, color=["g", "b", "r"])
+        coll = _plot_polygon_collection(ax, self.polygons, color=["g", "b", "r"])
         _check_colors(self.N, coll.get_facecolor(), ["g", "b", "r"])
         _check_colors(self.N, coll.get_edgecolor(), ["g", "b", "r"])
         ax.cla()
 
-        coll = plot_polygon_collection(
+        coll = _plot_polygon_collection(
             ax,
             self.polygons,
             color=[(0.5, 0.5, 0.5, 0.5), (0.1, 0.2, 0.3, 0.5), (0.4, 0.5, 0.6, 0.5)],
@@ -939,28 +950,31 @@ class TestPlotCollections:
         ax.cla()
 
         # only setting facecolor keeps default for edgecolor
-        coll = plot_polygon_collection(ax, self.polygons, facecolor="g")
+        coll = _plot_polygon_collection(ax, self.polygons, facecolor="g")
         _check_colors(self.N, coll.get_facecolor(), ["g"] * self.N)
         _check_colors(self.N, coll.get_edgecolor(), ["k"] * self.N)
         ax.cla()
 
         # custom facecolor and edgecolor
-        coll = plot_polygon_collection(ax, self.polygons, facecolor="g", edgecolor="r")
+        coll = _plot_polygon_collection(ax, self.polygons, facecolor="g", edgecolor="r")
         _check_colors(self.N, coll.get_facecolor(), ["g"] * self.N)
         _check_colors(self.N, coll.get_edgecolor(), ["r"] * self.N)
         ax.cla()
 
         # not a color
         with pytest.raises(TypeError):
-            plot_polygon_collection(ax, self.polygons, color="not color")
+            _plot_polygon_collection(ax, self.polygons, color="not color")
+        # check DeprecationWarning
+        with pytest.warns(DeprecationWarning):
+            plot_polygon_collection(ax, self.polygons)
 
     def test_polygons_values(self):
-        from geopandas.plotting import plot_polygon_collection
+        from geopandas.plotting import _plot_polygon_collection
 
         fig, ax = plt.subplots()
 
         # default colormap, edge is still black by default
-        coll = plot_polygon_collection(ax, self.polygons, self.values)
+        coll = _plot_polygon_collection(ax, self.polygons, self.values)
         fig.canvas.draw_idle()
         cmap = plt.get_cmap()
         exp_colors = cmap(np.arange(self.N) / (self.N - 1))
@@ -970,7 +984,7 @@ class TestPlotCollections:
         ax.cla()
 
         # specify colormap
-        coll = plot_polygon_collection(ax, self.polygons, self.values, cmap="RdBu")
+        coll = _plot_polygon_collection(ax, self.polygons, self.values, cmap="RdBu")
         fig.canvas.draw_idle()
         cmap = plt.get_cmap("RdBu")
         exp_colors = cmap(np.arange(self.N) / (self.N - 1))
@@ -978,7 +992,7 @@ class TestPlotCollections:
         ax.cla()
 
         # specify vmin/vmax
-        coll = plot_polygon_collection(ax, self.polygons, self.values, vmin=3, vmax=5)
+        coll = _plot_polygon_collection(ax, self.polygons, self.values, vmin=3, vmax=5)
         fig.canvas.draw_idle()
         cmap = plt.get_cmap()
         exp_colors = cmap([0])
@@ -986,7 +1000,7 @@ class TestPlotCollections:
         ax.cla()
 
         # override edgecolor
-        coll = plot_polygon_collection(ax, self.polygons, self.values, edgecolor="g")
+        coll = _plot_polygon_collection(ax, self.polygons, self.values, edgecolor="g")
         fig.canvas.draw_idle()
         cmap = plt.get_cmap()
         exp_colors = cmap(np.arange(self.N) / (self.N - 1))