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

Optimize mean_change (fixes issue #542) and correct documentation #574

Merged
merged 1 commit into from
Nov 16, 2019
Merged

Optimize mean_change (fixes issue #542) and correct documentation #574

merged 1 commit into from
Nov 16, 2019

Conversation

blu3r4y
Copy link
Contributor

@blu3r4y blu3r4y commented Oct 11, 2019

This uses a simpler formula for calculating mean_change that was already proposed in #542. The proof for this simplification follows. Moreover, I fixed the documentation, because we actually divide by n - 1 because a series with n elements only has n - 1 differences. I also added a few test cases to check some corner cases.

image

@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling 955479f on blu3r4y:optimize-mean-change into c3d35c4 on blue-yonder:master.

@dbarbier
Copy link
Contributor

Likewise mean_second_derivative_central could return (x[-1] - x[-2] - x[1] + x[0]) / (2 * (len(x) - 2))

@nils-braun
Copy link
Collaborator

Makes sense to do it like this! Thanks.
@dbarbier do you want to include the fix you mentioned as another PR?

@nils-braun nils-braun merged commit 54be2f1 into blue-yonder:master Nov 16, 2019
@dbarbier
Copy link
Contributor

Okay, will do, thanks for merging these PRs.

dbarbier added a commit to dbarbier/tsfresh that referenced this pull request Nov 16, 2019
This is similar to blue-yonder#542, mean_second_derivative_central has a simpler formula.
Also fix its docstring as had been done in blue-yonder#574.
@blu3r4y blu3r4y deleted the optimize-mean-change branch November 16, 2019 17:02
nils-braun pushed a commit that referenced this pull request Nov 17, 2019
This is similar to #542, mean_second_derivative_central has a simpler formula.
Also fix its docstring as had been done in #574.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants