Skip to content
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

Jump to bottom

BUG: .rolling() returns incorrect values when ts index is not nano seconds #55173

Merged
merged 20 commits into from Oct 26, 2023
Merged

BUG: .rolling() returns incorrect values when ts index is not nano seconds #55173

merged 20 commits into from Oct 26, 2023

Conversation

hkhojasteh
Copy link
Contributor

@hkhojasteh hkhojasteh commented Sep 16, 2023 •
edited by jorisvandenbossche

The bug is referred to and fixed inside the code. It needs to be generalised. @jorisvandenbossche we need to have the first feedback if we can generalise the data type with some utils or more generally. Done during the in-person sprint check.

@jorisvandenbossche jorisvandenbossche added the Sprints Sprint Pull Requests label Sep 16, 2023
@jorisvandenbossche jorisvandenbossche added this to the 2.1.1 milestone Sep 17, 2023
jorisvandenbossche
jorisvandenbossche reviewed Sep 17, 2023
View reviewed changes
doc/source/conf.py Outdated Show resolved Hide resolved
pandas/core/window/rolling.py Outdated Show resolved Hide resolved
pandas/tests/window/test_rolling.py Outdated Show resolved Hide resolved
pandas/tests/window/test_rolling.py Outdated Show resolved Hide resolved
@jorisvandenbossche jorisvandenbossche changed the title BUG: .rolling() returns incorrect values when ts index is not nano seconds #55106 BUG: .rolling() returns incorrect values when ts index is not nano seconds Sep 17, 2023
@jorisvandenbossche jorisvandenbossche added Bug Non-Nano datetime64/timedelta64 with non-nanosecond resolution labels Sep 17, 2023
@hkhojasteh
Copy link
Contributor Author

The test was parametrised, the rolling unit generalised and the version commit reverted :)
@jorisvandenbossche please let me know in case of any other changes

@lithomas1 lithomas1 modified the milestones: 2.1.1, 2.1.2 Sep 20, 2023
@lithomas1
Copy link
Member

Bumping off of 2.1.1.

@jorisvandenbossche jorisvandenbossche mentioned this pull request Sep 20, 2023
Closed
@hkhojasteh hkhojasteh mentioned this pull request Sep 22, 2023
Closed
2 tasks
@hkhojasteh
Copy link
Contributor Author

hkhojasteh commented Sep 22, 2023 •
edited

@lithomas1 This fix is also related to other subfunctions like max. More at #55026. The PR with the related tests is available.

@lithomas1
Copy link
Member

Can you add a whatsnew note for 2.1.2?

@mroeschke mroeschke added the Window rolling, ewma, expanding label Sep 29, 2023
@mroeschke mroeschke mentioned this pull request Sep 29, 2023
Closed
4 tasks
@hkhojasteh
Copy link
Contributor Author

Can you add a whatsnew note for 2.1.2?

Gladly added :)

@hkhojasteh @mroeschke
Update doc/source/whatsnew/v2.1.2.rst
2b6458e 
Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com>
jorisvandenbossche
jorisvandenbossche reviewed Oct 3, 2023
View reviewed changes
pandas/core/window/rolling.py Outdated Show resolved Hide resolved
pandas/core/window/rolling.py Outdated Show resolved Hide resolved
@hkhojasteh
Validating tz-aware data
a6a4d30 
Data conversion removed
Tests merged
@hkhojasteh
Copy link
Contributor Author

hkhojasteh commented Oct 3, 2023 •
edited

@jorisvandenbossche I have modified the validation function based on your suggestion. Tests are merged and the previous conversion has been removed. Please check it out and let me know

@hkhojasteh
Copy link
Contributor Author

@jorisvandenbossche I was about to ask how is the merge on this fix going on? Anything that I can help with?

@lithomas1
Copy link
Member

Tests are failing

FAILED pandas/tests/window/test_rolling.py::tests_empty_df_rolling[1s] - TypeError: cannot get datetime metadata from non-datetime type
FAILED pandas/tests/window/test_timeseries_window.py::TestRollingTS::test_invalid_window_nonfixed[2MS] - AssertionError: Regex pattern did not match.
 Regex: '\\<2 \\* MonthBegins\\> is a non-fixed frequency'
 Input: 'Value must be Timedelta, string, integer, float, timedelta or convertible, not MonthBegin'
FAILED pandas/tests/window/test_timeseries_window.py::TestRollingTS::test_invalid_window_nonfixed[freq1] - AssertionError: Regex pattern did not match.
 Regex: '\\<2 \\* MonthBegins\\> is a non-fixed frequency'
 Input: 'Value must be Timedelta, string, integer, float, timedelta or convertible, not MonthBegin'
FAILED pandas/tests/window/test_timeseries_window.py::TestRollingTS::test_rolling_on_empty - TypeError: cannot get datetime metadata from non-datetime type

@lithomas1 lithomas1 modified the milestones: 2.1.2, 2.1.3 Oct 25, 2023
@mroeschke mroeschke mentioned this pull request Oct 25, 2023
Closed
@mroeschke mroeschke merged commit fe17818 into pandas-dev:main Oct 26, 2023
36 of 39 checks passed
@meeseeksmachine meeseeksmachine mentioned this pull request Oct 26, 2023
Merged
meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Oct 26, 2023
@hkhojasteh @meeseeksmachine
Backport PR pandas-dev#55173: BUG: .rolling() returns incorrect value…
3e30241 
…s when ts index is not nano seconds
@mroeschke
Copy link
Member

Thanks @hkhojasteh and @jorisvandenbossche

jorisvandenbossche pushed a commit that referenced this pull request Oct 26, 2023
@meeseeksmachine @hkhojasteh
Backport PR #55173 on branch 2.1.x (BUG: .rolling() returns incorrect…
bceb84b 
… values when ts index is not nano seconds) (#55697)

Backport PR #55173: BUG: .rolling() returns incorrect values when ts index is not nano seconds

Co-authored-by: Hadi Abdi Khojasteh <hadiabdikhojasteh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbrockmendel jbrockmendel jbrockmendel left review comments

@mroeschke mroeschke mroeschke approved these changes

@jorisvandenbossche jorisvandenbossche Awaiting requested review from jorisvandenbossche

Assignees
No one assigned
Labels
Bug Non-Nano datetime64/timedelta64 with non-nanosecond resolution Sprints Sprint Pull Requests Window rolling, ewma, expanding
Projects
None yet
Milestone
2.1.2
5 participants
@hkhojasteh @lithomas1 @mroeschke @jorisvandenbossche @jbrockmendel