Skip to content

ENH: Raise useful error when iterating a Window #20996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 12, 2018
Merged

ENH: Raise useful error when iterating a Window #20996

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fe9e310
ENH: Raise useful error when iterating a Window
selik May 9, 2018
9fd89dc
Test that iteration raises NotImplemented
selik May 10, 2018
4ac67f0
Note window iteration raises NotImplemented
selik May 10, 2018
15224fe
switch % to .format
selik May 10, 2018
4b44b05
style change as per suggestion
selik May 12, 2018
1bcaa7f
fix testfunc parameters
selik May 12, 2018
6bd748d
s/cls/klass/g
selik May 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.23.0.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,7 @@ Other API Changes
- Constructing a Series from a list of length 1 no longer broadcasts this list when a longer index is specified (:issue:`19714`, :issue:`20391`).
- :func:`DataFrame.to_dict` with ``orient='index'`` no longer casts int columns to float for a DataFrame with only int and float columns (:issue:`18580`)
- A user-defined-function that is passed to :func:`Series.rolling().aggregate() <pandas.core.window.Rolling.aggregate>`, :func:`DataFrame.rolling().aggregate() <pandas.core.window.Rolling.aggregate>`, or its expanding cousins, will now *always* be passed a ``Series``, rather than a ``np.array``; ``.apply()`` only has the ``raw`` keyword, see :ref:`here <whatsnew_0230.enhancements.window_raw>`. This is consistent with the signatures of ``.aggregate()`` across pandas (:issue:`20584`)
- Rolling and Expanding types raise ``NotImplementedError`` upon iteration (:issue:`11704`).

.. _whatsnew_0230.deprecations:

Expand Down
4 changes: 4 additions & 0 deletions pandas/core/window.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ def __unicode__(self):
return "{klass} [{attrs}]".format(klass=self._window_type,
attrs=','.join(attrs))

def __iter__(self):
url = 'https://github.com/pandas-dev/pandas/issues/11704'
raise NotImplementedError('See issue #11704 {url}'.format(url=url))

def _get_index(self, index=None):
"""
Return index as ndarrays
Expand Down
16 changes: 16 additions & 0 deletions pandas/tests/test_window.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,14 @@ def test_multi_index_names(self):
tm.assert_index_equal(result.columns, df.columns)
assert result.index.names == [None, '1', '2']

@pytest.mark.parametrize('cls', [pd.Series, pd.DataFrame])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use "klass" instead of "cls"

Copy link
Contributor Author

@selik selik May 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly prefer cls in this case. Certain misspellings have a distasteful cultural undertone for me. Also, cls is an equally popular standard in other Python projects, if not more popular.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"klass" is most used in pandas so we should stick with that as the standard for now (thought admittedly not 100%). If you feel so inclined you can definitely bring up the conversation to standard as an issue or on the mailing list and get feedback.

Would be easier to change in one sweep that having each contributor implement their own standard

grep "parametrize.*klass" -r pandas/tests/ | wc -l
      38
grep "parametrize.*box" -r pandas/tests/ | wc -l
      12
grep "parametrize.*cls" -r pandas/tests/ | wc -l
      10

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll make that change separately.

def test_iter_raises(cls):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing "self" as first argument

# https://github.com/pandas-dev/pandas/issues/11704
# Iteration over a Window
obj = cls([1, 2, 3, 4])
with pytest.raises(NotImplementedError):
iter(obj.rolling(2))


class TestExpanding(Base):

Expand Down Expand Up @@ -590,6 +598,14 @@ def test_missing_minp_zero(self):
expected = pd.Series([np.nan])
tm.assert_series_equal(result, expected)

@pytest.mark.parametrize('cls', [pd.Series, pd.DataFrame])
def test_iter_raises(cls):
# https://github.com/pandas-dev/pandas/issues/11704
# Iteration over a Window
obj = cls([1, 2, 3, 4])
with pytest.raises(NotImplementedError):
iter(obj.expanding(2))


class TestEWM(Base):

Expand Down