From c985ee18b155238cad61e02d72a04888c487f024 Mon Sep 17 00:00:00 2001 From: Christian Chwala Date: Fri, 19 Jan 2018 00:25:31 +0100 Subject: [PATCH 1/7] Test that wrong string arguments raise an error --- pandas/tests/test_resample.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pandas/tests/test_resample.py b/pandas/tests/test_resample.py index e9a517605020a..0192aacf70355 100644 --- a/pandas/tests/test_resample.py +++ b/pandas/tests/test_resample.py @@ -963,6 +963,15 @@ def test_resample_basic(self): rng = date_range('1/1/2000 00:00:00', '1/1/2000 00:13:00', freq='min', name='index') s = Series(np.random.randn(14), index=rng) + + # Check that wrong keyword argument strings + with pytest.raises(AttributeError) as e_info: + s.resample('5min', label='righttt').mean() + with pytest.raises(AttributeError) as e_info: + s.resample('5min', closed='righttt').mean() + with pytest.raises(AttributeError) as e_info: + s.resample('5min', convention='starttt').mean() + result = s.resample('5min', closed='right', label='right').mean() exp_idx = date_range('1/1/2000', periods=4, freq='5min', name='index') From b05df9113010d1e9b3df873111875716a292431d Mon Sep 17 00:00:00 2001 From: Christian Chwala Date: Fri, 19 Jan 2018 09:19:07 +0100 Subject: [PATCH 2/7] Added a separate test according to change request --- pandas/tests/test_resample.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/pandas/tests/test_resample.py b/pandas/tests/test_resample.py index 0192aacf70355..1162331fd246a 100644 --- a/pandas/tests/test_resample.py +++ b/pandas/tests/test_resample.py @@ -964,14 +964,6 @@ def test_resample_basic(self): name='index') s = Series(np.random.randn(14), index=rng) - # Check that wrong keyword argument strings - with pytest.raises(AttributeError) as e_info: - s.resample('5min', label='righttt').mean() - with pytest.raises(AttributeError) as e_info: - s.resample('5min', closed='righttt').mean() - with pytest.raises(AttributeError) as e_info: - s.resample('5min', convention='starttt').mean() - result = s.resample('5min', closed='right', label='right').mean() exp_idx = date_range('1/1/2000', periods=4, freq='5min', name='index') @@ -994,6 +986,19 @@ def test_resample_basic(self): expect = s.groupby(grouper).agg(lambda x: x[-1]) assert_series_equal(result, expect) + def test_resample_string_kwargs(self): + rng = date_range('1/1/2000 00:00:00', '1/1/2000 00:13:00', freq='min', + name='index') + s = Series(np.random.randn(14), index=rng) + + # Check that wrong keyword argument strings raise an error + with pytest.raises(AttributeError) as e_info: + s.resample('5min', label='righttt').mean() + with pytest.raises(AttributeError) as e_info: + s.resample('5min', closed='righttt').mean() + with pytest.raises(AttributeError) as e_info: + s.resample('5min', convention='starttt').mean() + def test_resample_how(self): rng = date_range('1/1/2000 00:00:00', '1/1/2000 00:13:00', freq='min', name='index') From 1149226e5676bdb38df230bb6bad3a7b8b65e3f1 Mon Sep 17 00:00:00 2001 From: Christian Chwala Date: Fri, 19 Jan 2018 13:15:24 +0100 Subject: [PATCH 3/7] Check validity of string arguments for `label`, `closed` and `convention` --- doc/source/whatsnew/v0.23.0.txt | 2 +- pandas/core/resample.py | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.23.0.txt b/doc/source/whatsnew/v0.23.0.txt index 2f3b62febed7d..4bbbc61a64357 100644 --- a/doc/source/whatsnew/v0.23.0.txt +++ b/doc/source/whatsnew/v0.23.0.txt @@ -485,7 +485,7 @@ Groupby/Resample/Rolling - Bug when grouping by a single column and aggregating with a class like ``list`` or ``tuple`` (:issue:`18079`) - Fixed regression in :func:`DataFrame.groupby` which would not emit an error when called with a tuple key not in the index (:issue:`18798`) -- +- Bug in :func:`DataFrame.resample` which silently ignored unsupported (or misstyped) options for `label`, `closed` and `convention` (:issue:`19303`) - Sparse diff --git a/pandas/core/resample.py b/pandas/core/resample.py index 5447ce7470b9d..b260574756201 100644 --- a/pandas/core/resample.py +++ b/pandas/core/resample.py @@ -1061,6 +1061,16 @@ class TimeGrouper(Grouper): def __init__(self, freq='Min', closed=None, label=None, how='mean', axis=0, fill_method=None, limit=None, loffset=None, kind=None, convention=None, base=0, **kwargs): + # Check for correctness of the keyword arguments which would + # otherwise silently use the default if misspelled + if label not in {None, 'left', 'right'}: + raise AttributeError('Unsupported value %s for `label`' % label) + if closed not in {None, 'left', 'right'}: + raise AttributeError('Unsupported value %s for `closed`' % closed) + if convention not in {None, 'start', 'end', 'e', 's'}: + raise AttributeError('Unsupported value %s for `convention`' + % convention) + freq = to_offset(freq) end_types = set(['M', 'A', 'Q', 'BM', 'BA', 'BQ', 'W']) From 24bcb9843971df2b735dd607857aa5861926f6ed Mon Sep 17 00:00:00 2001 From: Christian Chwala Date: Fri, 19 Jan 2018 15:21:59 +0100 Subject: [PATCH 4/7] Updates according to code review --- doc/source/whatsnew/v0.23.0.txt | 2 +- pandas/core/resample.py | 6 +++--- pandas/tests/test_resample.py | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/source/whatsnew/v0.23.0.txt b/doc/source/whatsnew/v0.23.0.txt index 4bbbc61a64357..3fad33d208eb9 100644 --- a/doc/source/whatsnew/v0.23.0.txt +++ b/doc/source/whatsnew/v0.23.0.txt @@ -485,7 +485,7 @@ Groupby/Resample/Rolling - Bug when grouping by a single column and aggregating with a class like ``list`` or ``tuple`` (:issue:`18079`) - Fixed regression in :func:`DataFrame.groupby` which would not emit an error when called with a tuple key not in the index (:issue:`18798`) -- Bug in :func:`DataFrame.resample` which silently ignored unsupported (or misstyped) options for `label`, `closed` and `convention` (:issue:`19303`) +- Bug in :func:`DataFrame.resample` which silently ignored unsupported (or mistyped) options for ``label``, ``closed`` and ``convention`` (:issue:`19303`) - Sparse diff --git a/pandas/core/resample.py b/pandas/core/resample.py index b260574756201..dff5920fb5f3b 100644 --- a/pandas/core/resample.py +++ b/pandas/core/resample.py @@ -1064,11 +1064,11 @@ def __init__(self, freq='Min', closed=None, label=None, how='mean', # Check for correctness of the keyword arguments which would # otherwise silently use the default if misspelled if label not in {None, 'left', 'right'}: - raise AttributeError('Unsupported value %s for `label`' % label) + raise ValueError('Unsupported value %s for `label`' % label) if closed not in {None, 'left', 'right'}: - raise AttributeError('Unsupported value %s for `closed`' % closed) + raise ValueError('Unsupported value %s for `closed`' % closed) if convention not in {None, 'start', 'end', 'e', 's'}: - raise AttributeError('Unsupported value %s for `convention`' + raise ValueError('Unsupported value %s for `convention`' % convention) freq = to_offset(freq) diff --git a/pandas/tests/test_resample.py b/pandas/tests/test_resample.py index 1162331fd246a..b02153374968b 100644 --- a/pandas/tests/test_resample.py +++ b/pandas/tests/test_resample.py @@ -992,11 +992,11 @@ def test_resample_string_kwargs(self): s = Series(np.random.randn(14), index=rng) # Check that wrong keyword argument strings raise an error - with pytest.raises(AttributeError) as e_info: + with pytest.raises(ValueError) as e_info: s.resample('5min', label='righttt').mean() - with pytest.raises(AttributeError) as e_info: + with pytest.raises(ValueError) as e_info: s.resample('5min', closed='righttt').mean() - with pytest.raises(AttributeError) as e_info: + with pytest.raises(ValueError) as e_info: s.resample('5min', convention='starttt').mean() def test_resample_how(self): From 9eaae6c7dc2f645f4967e757d7b25b1cb8db14b7 Mon Sep 17 00:00:00 2001 From: Christian Chwala Date: Fri, 19 Jan 2018 15:26:30 +0100 Subject: [PATCH 5/7] pep8... --- pandas/core/resample.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/resample.py b/pandas/core/resample.py index dff5920fb5f3b..aa0a4457d1a1b 100644 --- a/pandas/core/resample.py +++ b/pandas/core/resample.py @@ -1069,7 +1069,7 @@ def __init__(self, freq='Min', closed=None, label=None, how='mean', raise ValueError('Unsupported value %s for `closed`' % closed) if convention not in {None, 'start', 'end', 'e', 's'}: raise ValueError('Unsupported value %s for `convention`' - % convention) + % convention) freq = to_offset(freq) From 432332e3f38f25c85dc543465714fe50e56b14f0 Mon Sep 17 00:00:00 2001 From: Christian Chwala Date: Sat, 20 Jan 2018 21:55:12 +0100 Subject: [PATCH 6/7] change string formating, fixed lint problem, mentioned issue number --- pandas/core/resample.py | 8 ++++---- pandas/tests/test_resample.py | 7 ++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pandas/core/resample.py b/pandas/core/resample.py index aa0a4457d1a1b..f6250e70eed91 100644 --- a/pandas/core/resample.py +++ b/pandas/core/resample.py @@ -1064,12 +1064,12 @@ def __init__(self, freq='Min', closed=None, label=None, how='mean', # Check for correctness of the keyword arguments which would # otherwise silently use the default if misspelled if label not in {None, 'left', 'right'}: - raise ValueError('Unsupported value %s for `label`' % label) + raise ValueError('Unsupported value {} for `label`'.format(label)) if closed not in {None, 'left', 'right'}: - raise ValueError('Unsupported value %s for `closed`' % closed) + raise ValueError('Unsupported value {} for `closed`'.format(closed)) if convention not in {None, 'start', 'end', 'e', 's'}: - raise ValueError('Unsupported value %s for `convention`' - % convention) + raise ValueError('Unsupported value {} for `convention`' + .format(convention)) freq = to_offset(freq) diff --git a/pandas/tests/test_resample.py b/pandas/tests/test_resample.py index b02153374968b..6f77e7854cf76 100644 --- a/pandas/tests/test_resample.py +++ b/pandas/tests/test_resample.py @@ -987,16 +987,17 @@ def test_resample_basic(self): assert_series_equal(result, expect) def test_resample_string_kwargs(self): + # Test for issue #19303 rng = date_range('1/1/2000 00:00:00', '1/1/2000 00:13:00', freq='min', name='index') s = Series(np.random.randn(14), index=rng) # Check that wrong keyword argument strings raise an error - with pytest.raises(ValueError) as e_info: + with pytest.raises(ValueError): s.resample('5min', label='righttt').mean() - with pytest.raises(ValueError) as e_info: + with pytest.raises(ValueError): s.resample('5min', closed='righttt').mean() - with pytest.raises(ValueError) as e_info: + with pytest.raises(ValueError): s.resample('5min', convention='starttt').mean() def test_resample_how(self): From c02676758f499241a126473c7fc735e102637d21 Mon Sep 17 00:00:00 2001 From: Jeff Reback Date: Sun, 21 Jan 2018 10:46:33 -0500 Subject: [PATCH 7/7] lint --- pandas/core/resample.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pandas/core/resample.py b/pandas/core/resample.py index f6250e70eed91..04f5c124deccc 100644 --- a/pandas/core/resample.py +++ b/pandas/core/resample.py @@ -1066,7 +1066,8 @@ def __init__(self, freq='Min', closed=None, label=None, how='mean', if label not in {None, 'left', 'right'}: raise ValueError('Unsupported value {} for `label`'.format(label)) if closed not in {None, 'left', 'right'}: - raise ValueError('Unsupported value {} for `closed`'.format(closed)) + raise ValueError('Unsupported value {} for `closed`'.format( + closed)) if convention not in {None, 'start', 'end', 'e', 's'}: raise ValueError('Unsupported value {} for `convention`' .format(convention))