From bd01a378d6965ae0d1e95d0bddf79332086ad809 Mon Sep 17 00:00:00 2001 From: MasonGallo Date: Fri, 8 Dec 2017 11:40:28 -0500 Subject: [PATCH 1/4] Raise ValueError if improper argument given to df.plot --- pandas/plotting/_core.py | 6 +++++- pandas/tests/plotting/test_frame.py | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index adaaa206edadd..e23f85dac5453 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -1680,7 +1680,7 @@ def _plot(data, x=None, y=None, subplots=False, else: raise ValueError("%r is not a valid plot kind" % kind) - from pandas import DataFrame + from pandas import DataFrame, Series if kind in _dataframe_kinds: if isinstance(data, DataFrame): plot_obj = klass(data, x=x, y=y, subplots=subplots, ax=ax, @@ -1706,11 +1706,15 @@ def _plot(data, x=None, y=None, subplots=False, if x is not None: if is_integer(x) and not data.columns.holds_integer(): x = data.columns[x] + elif not isinstance(data[x], Series): + raise ValueError("x must be a label or position") data = data.set_index(x) if y is not None: if is_integer(y) and not data.columns.holds_integer(): y = data.columns[y] + elif not isinstance(data[y], Series): + raise ValueError("y must be a label or position") label = kwds['label'] if 'label' in kwds else y series = data[y].copy() # Don't modify series.name = label diff --git a/pandas/tests/plotting/test_frame.py b/pandas/tests/plotting/test_frame.py index 5c72d778a1220..88005a5bb3589 100644 --- a/pandas/tests/plotting/test_frame.py +++ b/pandas/tests/plotting/test_frame.py @@ -2170,6 +2170,15 @@ def test_invalid_kind(self): with pytest.raises(ValueError): df.plot(kind='aasdf') + def test_invalid_xy_args(self): + df = DataFrame({"A": [1, 2], 'B': [3, 4], 'C': [5, 6]}) + bad_arg = ['B', 'C'] + valid_arg = 'A' + with pytest.raises(ValueError): + df.plot(x=bad_arg, y=valid_arg) + with pytest.raises(ValueError): + df.plot(x=valid_arg, y=bad_arg) + @pytest.mark.slow def test_hexbin_basic(self): df = self.hexbin_df From e190aa3cbdec4ec7c42fb116a58b31fa7f9167f6 Mon Sep 17 00:00:00 2001 From: MasonGallo Date: Fri, 8 Dec 2017 12:59:39 -0500 Subject: [PATCH 2/4] update whatsnew 0.22 --- doc/source/whatsnew/v0.22.0.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/source/whatsnew/v0.22.0.txt b/doc/source/whatsnew/v0.22.0.txt index c3a0e3599a0f9..a0345e41cab9f 100644 --- a/doc/source/whatsnew/v0.22.0.txt +++ b/doc/source/whatsnew/v0.22.0.txt @@ -188,6 +188,7 @@ Other API Changes - :func:`pandas.DataFrame.merge` no longer casts a ``float`` column to ``object`` when merging on ``int`` and ``float`` columns (:issue:`16572`) - The default NA value for :class:`UInt64Index` has changed from 0 to ``NaN``, which impacts methods that mask with NA, such as ``UInt64Index.where()`` (:issue:`18398`) - Refactored ``setup.py`` to use ``find_packages`` instead of explicitly listing out all subpackages (:issue:`18535`) +- :func: `DataFrame.plot` now raises a ``ValueError`` when the ``x`` or ``y`` argument is improperly formed (:issue:`18671`) .. _whatsnew_0220.deprecations: From c5f3e2a2f3cc11157dd9ea9e3b767e259f4864c0 Mon Sep 17 00:00:00 2001 From: MasonGallo Date: Sat, 9 Dec 2017 16:30:31 -0500 Subject: [PATCH 3/4] address fdbck --- doc/source/whatsnew/v0.22.0.txt | 3 +-- pandas/plotting/_core.py | 6 +++--- pandas/tests/plotting/test_frame.py | 24 ++++++++++++++++++------ 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/doc/source/whatsnew/v0.22.0.txt b/doc/source/whatsnew/v0.22.0.txt index a0345e41cab9f..5a5d6e92c21e8 100644 --- a/doc/source/whatsnew/v0.22.0.txt +++ b/doc/source/whatsnew/v0.22.0.txt @@ -188,7 +188,6 @@ Other API Changes - :func:`pandas.DataFrame.merge` no longer casts a ``float`` column to ``object`` when merging on ``int`` and ``float`` columns (:issue:`16572`) - The default NA value for :class:`UInt64Index` has changed from 0 to ``NaN``, which impacts methods that mask with NA, such as ``UInt64Index.where()`` (:issue:`18398`) - Refactored ``setup.py`` to use ``find_packages`` instead of explicitly listing out all subpackages (:issue:`18535`) -- :func: `DataFrame.plot` now raises a ``ValueError`` when the ``x`` or ``y`` argument is improperly formed (:issue:`18671`) .. _whatsnew_0220.deprecations: @@ -283,7 +282,7 @@ I/O Plotting ^^^^^^^^ -- +- :func: `DataFrame.plot` now raises a ``ValueError`` when the ``x`` or ``y`` argument is improperly formed (:issue:`18671`) - - diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index e23f85dac5453..c4fb8731f2178 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -1680,7 +1680,7 @@ def _plot(data, x=None, y=None, subplots=False, else: raise ValueError("%r is not a valid plot kind" % kind) - from pandas import DataFrame, Series + from pandas import DataFrame if kind in _dataframe_kinds: if isinstance(data, DataFrame): plot_obj = klass(data, x=x, y=y, subplots=subplots, ax=ax, @@ -1706,14 +1706,14 @@ def _plot(data, x=None, y=None, subplots=False, if x is not None: if is_integer(x) and not data.columns.holds_integer(): x = data.columns[x] - elif not isinstance(data[x], Series): + elif not isinstance(data[x], ABCSeries): raise ValueError("x must be a label or position") data = data.set_index(x) if y is not None: if is_integer(y) and not data.columns.holds_integer(): y = data.columns[y] - elif not isinstance(data[y], Series): + elif not isinstance(data[y], ABCSeries): raise ValueError("y must be a label or position") label = kwds['label'] if 'label' in kwds else y series = data[y].copy() # Don't modify diff --git a/pandas/tests/plotting/test_frame.py b/pandas/tests/plotting/test_frame.py index 88005a5bb3589..de4219cb9afe5 100644 --- a/pandas/tests/plotting/test_frame.py +++ b/pandas/tests/plotting/test_frame.py @@ -12,6 +12,7 @@ from pandas import (Series, DataFrame, MultiIndex, PeriodIndex, date_range, bdate_range) from pandas.core.dtypes.api import is_list_like +from pandas.core.dtypes.generic import ABCDataFrame from pandas.compat import range, lrange, lmap, lzip, u, zip, PY3 from pandas.io.formats.printing import pprint_thing import pandas.util.testing as tm @@ -2170,14 +2171,25 @@ def test_invalid_kind(self): with pytest.raises(ValueError): df.plot(kind='aasdf') - def test_invalid_xy_args(self): - df = DataFrame({"A": [1, 2], 'B': [3, 4], 'C': [5, 6]}) - bad_arg = ['B', 'C'] - valid_arg = 'A' + @pytest.mark.parametrize("x,y", [ + (['B', 'C'], 'A'), + ('A', ['B', 'C']) + ]) + def test_invalid_xy_args(self, x, y): + # GH 18671 + df = ABCDataFrame({"A": [1, 2], 'B': [3, 4], 'C': [5, 6]}) with pytest.raises(ValueError): - df.plot(x=bad_arg, y=valid_arg) + df.plot(x=x, y=y) + + @pytest.mark.parametrize("x,y", [ + ('A', 'B'), + ('B', 'A') + ]) + def test_invalid_xy_args_dup_cols(self, x, y): + # GH 18671 + df = ABCDataFrame([[1, 3, 5], [2, 4, 6]], columns=list('AAB')) with pytest.raises(ValueError): - df.plot(x=valid_arg, y=bad_arg) + df.plot(x=x, y=y) @pytest.mark.slow def test_hexbin_basic(self): From aea83836926f0f4a3a2cc03f7b88d65179d3fade Mon Sep 17 00:00:00 2001 From: MasonGallo Date: Sun, 10 Dec 2017 00:41:00 -0500 Subject: [PATCH 4/4] hopefully correct abcdataframe --- pandas/plotting/_core.py | 9 ++++----- pandas/tests/plotting/test_frame.py | 5 ++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pandas/plotting/_core.py b/pandas/plotting/_core.py index c4fb8731f2178..9d74a308f79c8 100644 --- a/pandas/plotting/_core.py +++ b/pandas/plotting/_core.py @@ -19,7 +19,7 @@ is_number, is_hashable, is_iterator) -from pandas.core.dtypes.generic import ABCSeries +from pandas.core.dtypes.generic import ABCSeries, ABCDataFrame from pandas.core.common import AbstractMethodError, _try_sort, _any_not_none from pandas.core.generic import _shared_docs, _shared_doc_kwargs @@ -1680,9 +1680,8 @@ def _plot(data, x=None, y=None, subplots=False, else: raise ValueError("%r is not a valid plot kind" % kind) - from pandas import DataFrame if kind in _dataframe_kinds: - if isinstance(data, DataFrame): + if isinstance(data, ABCDataFrame): plot_obj = klass(data, x=x, y=y, subplots=subplots, ax=ax, kind=kind, **kwds) else: @@ -1690,7 +1689,7 @@ def _plot(data, x=None, y=None, subplots=False, % kind) elif kind in _series_kinds: - if isinstance(data, DataFrame): + if isinstance(data, ABCDataFrame): if y is None and subplots is False: msg = "{0} requires either y column or 'subplots=True'" raise ValueError(msg.format(kind)) @@ -1702,7 +1701,7 @@ def _plot(data, x=None, y=None, subplots=False, data.index.name = y plot_obj = klass(data, subplots=subplots, ax=ax, kind=kind, **kwds) else: - if isinstance(data, DataFrame): + if isinstance(data, ABCDataFrame): if x is not None: if is_integer(x) and not data.columns.holds_integer(): x = data.columns[x] diff --git a/pandas/tests/plotting/test_frame.py b/pandas/tests/plotting/test_frame.py index de4219cb9afe5..3b3f6666340b8 100644 --- a/pandas/tests/plotting/test_frame.py +++ b/pandas/tests/plotting/test_frame.py @@ -12,7 +12,6 @@ from pandas import (Series, DataFrame, MultiIndex, PeriodIndex, date_range, bdate_range) from pandas.core.dtypes.api import is_list_like -from pandas.core.dtypes.generic import ABCDataFrame from pandas.compat import range, lrange, lmap, lzip, u, zip, PY3 from pandas.io.formats.printing import pprint_thing import pandas.util.testing as tm @@ -2177,7 +2176,7 @@ def test_invalid_kind(self): ]) def test_invalid_xy_args(self, x, y): # GH 18671 - df = ABCDataFrame({"A": [1, 2], 'B': [3, 4], 'C': [5, 6]}) + df = DataFrame({"A": [1, 2], 'B': [3, 4], 'C': [5, 6]}) with pytest.raises(ValueError): df.plot(x=x, y=y) @@ -2187,7 +2186,7 @@ def test_invalid_xy_args(self, x, y): ]) def test_invalid_xy_args_dup_cols(self, x, y): # GH 18671 - df = ABCDataFrame([[1, 3, 5], [2, 4, 6]], columns=list('AAB')) + df = DataFrame([[1, 3, 5], [2, 4, 6]], columns=list('AAB')) with pytest.raises(ValueError): df.plot(x=x, y=y)