Skip to content

Conversation

hkhojasteh
Copy link
Contributor

@hkhojasteh hkhojasteh commented Sep 16, 2023

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 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
Contributor

Bumping off of 2.1.1.

@hkhojasteh
Copy link
Contributor Author

hkhojasteh commented Sep 22, 2023

@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
Contributor

Can you add a whatsnew note for 2.1.2?

@hkhojasteh
Copy link
Contributor Author

Can you add a whatsnew note for 2.1.2?

Gladly added :)

Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com>
@hkhojasteh hkhojasteh requested a review from mroeschke October 2, 2023 17:48
@hkhojasteh hkhojasteh requested a review from mroeschke October 3, 2023 00:26
Data conversion removed
Tests merged
@hkhojasteh
Copy link
Contributor Author

hkhojasteh commented Oct 3, 2023

@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
Contributor

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
@mroeschke mroeschke merged commit fe17818 into pandas-dev:main Oct 26, 2023
meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Oct 26, 2023
@mroeschke
Copy link
Member

Thanks @hkhojasteh and @jorisvandenbossche

jorisvandenbossche pushed a commit that referenced this pull request Oct 26, 2023
… 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
Labels
Bug Non-Nano datetime64/timedelta64 with non-nanosecond resolution Sprints Sprint Pull Requests Window rolling, ewma, expanding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: .rolling() returns incorrect values when ts index is not nano seconds BUG: Rolling pd.date_range incorrect for unit='s'
5 participants