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.

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

BUG: Fixed regression in rolling.skew and rolling.kurt modifying object #38909

Merged
merged 3 commits into from
Jan 4, 2021

Conversation

phofl
Copy link
Member

@phofl phofl commented Jan 2, 2021

Alternative would be np.copy before releasing gil. In this case we would have to touch values_copy twice in case of min_val - mean_val > -1e5

@phofl phofl added Regression Functionality that used to work in a prior pandas version Window rolling, ewma, expanding labels Jan 2, 2021
@simonjayhawkins simonjayhawkins added this to the 1.2.1 milestone Jan 3, 2021
@@ -504,6 +504,7 @@ def roll_skew(ndarray[float64_t] values, ndarray[int64_t] start,
)
output = np.empty(N, dtype=float)
min_val = np.nanmin(values)
values_copy = np.empty(N, dtype=float)
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't you simply copy values to begin with? then all of these additions are not needed

Copy link
Member Author

Choose a reason for hiding this comment

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

Was not sure which would be faster. When we do a copy, we have to touch the values again in case of

min_val - mean_val > -1e5

Changed it

@jreback jreback merged commit 8f26de1 into pandas-dev:master Jan 4, 2021
@jreback
Copy link
Contributor

jreback commented Jan 4, 2021

thanks @phofl

@jreback
Copy link
Contributor

jreback commented Jan 4, 2021

@meeseeksdev backport 1.2.x

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Regression Functionality that used to work in a prior pandas version Window rolling, ewma, expanding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: pandas 1.2.0 df.rolling().aggregate('skew') modified original data
3 participants