-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
RFC: Add more scipy interpolations methods #5067
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
Conversation
| "cubic", | ||
| # "next", | ||
| # "previous", | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we would add next and previous to the pandas compat tests as well. However, these interpolation methods are not wrapped by pandas yet, so they currently fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pandas has this functionality in pad/backfill for fillna() rather than in interpolate. That could be a good method to try for the tests here.
dcherian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice first PR @meggart
I have one small suggestion regarding the tests. A whats-new entry would be nice to advertise these cool new capabilities.
| "slinear", | ||
| "quadratic", | ||
| "cubic", | ||
| "previous", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should check that the output matches that from scipy since we can't use pandas for that
Can you add that check here?
This extends the list of applicable interpolation methods from scipy's
interp1dto match the methods listed in https://docs.scipy.org/doc/scipy/reference/generated/scipy.interpolate.interp1d.htmlpre-commit run --all-fileswhats-new.rstIs such a small change worth being included in
whats-new.rst?