BUG: Fix rolling functions with variable windows on decreasing index #32386

leftys · 2020-03-01T15:52:32Z

Closes BUG: Rolling functions doesn't work on decreasing variable index #32385
tests updated & passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

leftys · 2020-03-01T18:54:23Z

pandas/tests/window/test_timeseries_window.py

        expected = DataFrame(
-            {"column": [3.0, 3.0, 3.0, 2.0, 1.0]}, index=reversed(index)
+            {"column": [3.0, 3.0, 4.0, 4.0, 6.0]}, index=index


The original expected value was wrong: #28297 (comment).

… index and update tests CLN: black reformatting

doc/source/whatsnew/v1.0.2.rst

jorisvandenbossche · 2020-03-02T22:50:49Z

cc @mroeschke are you familiar with this code?

I can confirm that indeed the current output (and tested behaviour) is wrong

mroeschke · 2020-03-03T01:15:12Z

Only familiar from a refactor standpoint. I didn't write the original window bounds calculations.

mroeschke · 2020-03-03T01:15:36Z

pandas/_libs/window/indexers.pyx

@@ -44,6 +44,7 @@ def calculate_variable_window_bounds(
    cdef:
        bint left_closed = False
        bint right_closed = False
+        int index_growth_sign = +1


+1 -> 1

mroeschke · 2020-03-03T01:16:07Z

pandas/_libs/window/indexers.pyx

@@ -58,6 +59,9 @@ def calculate_variable_window_bounds(
    if closed in ['left', 'both']:
        left_closed = True

+    if index[num_values-1] < index[0]:


index[num_values-1] -> index[num_values - 1]

WillAyd · 2020-03-03T03:01:07Z

pandas/tests/window/test_timeseries_window.py

        ]

-        df = DataFrame({"column": [3, 4, 4, 2, 1]}, index=reversed(index))


Wouldn't this have been working previously with a decreasing index?

Not fully sure what the question is, but: on 0.25 we raised an error for decreasing index. For 1.0, rolling with a decreasing index was enabled, but gave wrong output (see eg #32385 for what the output of this should actually be)

v1.0 would return [3,3,3,2,1] for this case. The rolling window covered the whole series.

jreback

lgtm. a question.

pandas/_libs/window/indexers.pyx

pandas/tests/window/test_timeseries_window.py

TomAugspurger · 2020-03-09T15:39:08Z

@leftys can you respond to #32386 (review)? Trying to get 1.0.2 out soon.

leftys · 2020-03-09T18:42:58Z

@leftys can you respond to #32386 (review)? Trying to get 1.0.2 out soon.

@TomAugspurger done

jreback · 2020-03-11T01:51:10Z

thanks @leftys very nice!

jreback · 2020-03-11T01:53:19Z

@meeseeksdev backport 1.0.x

…e windows on decreasing index

…on decreasing index (#32606) Co-authored-by: Jan Škoda <[email protected]>

…andas-dev#32386)

leftys mentioned this pull request Mar 1, 2020

fix Rolling for multi-index and reversed index. #28297

Merged

6 tasks

leftys changed the title ~~Fix rolling functions on decreasing variable index~~ Fix rolling functions with variable windows on decreasing index Mar 1, 2020

leftys commented Mar 1, 2020

View reviewed changes

leftys changed the title ~~Fix rolling functions with variable windows on decreasing index~~ BUG: Fix rolling functions with variable windows on decreasing index Mar 1, 2020

leftys added 3 commits March 2, 2020 09:07

BUG: fix calculate_variable_window_bounds with monotonic decreasing…

e5bef9c

… index and update tests CLN: black reformatting

DOC: docstrings fix

5bddb9b

DOC: whatsnew update

ff24fab

jorisvandenbossche reviewed Mar 2, 2020

View reviewed changes

doc/source/whatsnew/v1.0.2.rst Outdated Show resolved Hide resolved

mroeschke reviewed Mar 3, 2020

View reviewed changes

WillAyd reviewed Mar 3, 2020

View reviewed changes

jorisvandenbossche added this to the 1.0.2 milestone Mar 3, 2020

jorisvandenbossche added Bug Window rolling, ewma, expanding labels Mar 3, 2020

CLN: style fixes

b22b3ed

leftys requested review from jorisvandenbossche and mroeschke March 3, 2020 08:38

jreback requested changes Mar 4, 2020

View reviewed changes

pandas/_libs/window/indexers.pyx Show resolved Hide resolved

pandas/tests/window/test_timeseries_window.py Show resolved Hide resolved

TST: test rolling on empty dataframe

281152a

leftys requested a review from jreback March 9, 2020 18:43

TomAugspurger linked an issue Mar 10, 2020 that may be closed by this pull request

BUG: Rolling functions doesn't work on decreasing variable index #32385

Closed

jreback approved these changes Mar 11, 2020

View reviewed changes

jreback merged commit 8bc471a into pandas-dev:master Mar 11, 2020

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Mar 11, 2020

Backport PR pandas-dev#32386: BUG: Fix rolling functions with variabl…

4d77973

…e windows on decreasing index

meeseeksmachine mentioned this pull request Mar 11, 2020

Backport PR #32386 on branch 1.0.x (BUG: Fix rolling functions with variable windows on decreasing index) #32606

Merged

jreback added Still Needs Manual Backport and removed Still Needs Manual Backport labels Mar 11, 2020

jreback pushed a commit that referenced this pull request Mar 11, 2020

Backport PR #32386: BUG: Fix rolling functions with variable windows …

daae221

…on decreasing index (#32606) Co-authored-by: Jan Škoda <[email protected]>

SeeminSyed pushed a commit to CSCD01-team01/pandas that referenced this pull request Mar 22, 2020

BUG: Fix rolling functions with variable windows on decreasing index (p…

c7d0915

…andas-dev#32386)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Fix rolling functions with variable windows on decreasing index #32386

BUG: Fix rolling functions with variable windows on decreasing index #32386

leftys commented Mar 1, 2020 •

edited by TomAugspurger

Loading

leftys Mar 1, 2020

jorisvandenbossche commented Mar 2, 2020

mroeschke commented Mar 3, 2020

mroeschke Mar 3, 2020

leftys Mar 3, 2020

mroeschke Mar 3, 2020

leftys Mar 3, 2020

WillAyd Mar 3, 2020

jorisvandenbossche Mar 3, 2020

leftys Mar 3, 2020 •

edited

Loading

jreback left a comment

TomAugspurger commented Mar 9, 2020

leftys commented Mar 9, 2020

jreback commented Mar 11, 2020

jreback commented Mar 11, 2020

		]

		df = DataFrame({"column": [3, 4, 4, 2, 1]}, index=reversed(index))

BUG: Fix rolling functions with variable windows on decreasing index #32386

BUG: Fix rolling functions with variable windows on decreasing index #32386

Conversation

leftys commented Mar 1, 2020 • edited by TomAugspurger Loading

leftys Mar 1, 2020

Choose a reason for hiding this comment

jorisvandenbossche commented Mar 2, 2020

mroeschke commented Mar 3, 2020

mroeschke Mar 3, 2020

Choose a reason for hiding this comment

leftys Mar 3, 2020

Choose a reason for hiding this comment

mroeschke Mar 3, 2020

Choose a reason for hiding this comment

leftys Mar 3, 2020

Choose a reason for hiding this comment

WillAyd Mar 3, 2020

Choose a reason for hiding this comment

jorisvandenbossche Mar 3, 2020

Choose a reason for hiding this comment

leftys Mar 3, 2020 • edited Loading

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

TomAugspurger commented Mar 9, 2020

leftys commented Mar 9, 2020

jreback commented Mar 11, 2020

jreback commented Mar 11, 2020

leftys commented Mar 1, 2020 •

edited by TomAugspurger

Loading

leftys Mar 3, 2020 •

edited

Loading