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

scipy 0.17.1 piecewise cubic hermite interpolation does not return roots #6357

Closed
kaufmanno opened this issue Jul 7, 2016 · 3 comments · Fixed by #6462
Closed

scipy 0.17.1 piecewise cubic hermite interpolation does not return roots #6357

kaufmanno opened this issue Jul 7, 2016 · 3 comments · Fixed by #6462
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected good first issue Good topic for first contributor pull requests, with a relatively straightforward solution scipy.interpolate
Milestone

Comments

@kaufmanno
Copy link

The root method fails and returns an error with the scipy PchipInterpolator piecewise cubic hermite interpolation.
see this question on stackoverflow

@ev-br ev-br added scipy.interpolate defect A clear bug or issue that prevents SciPy from being installed or used as expected good first issue Good topic for first contributor pull requests, with a relatively straightforward solution labels Jul 7, 2016
@ev-br
Copy link
Member

ev-br commented Jul 7, 2016

Thanks for reporting it. The fix seems to be reasonably easy: self._bpoly in the roots method should be just self. And it needs a test --- the very reason the bug slipped through the pchip refactor is that it was not tested.

Would you be interested in sending a PR?

@kaufmanno
Copy link
Author

Dear Evgeni,

First of all, thank you for your quick and useful reply to my question!

I am willing to help even if I am still learning to work in large collaborative environments and am not yet always sure of the right way to do it...

Should I first clone the entire scipy repository (0.17.1 ?), then make the changes to the roots method and finally make a pull request on github?

Cheers,

Olivier

P.S. I am currently working on this project on my spare time, and it could take me a few days before this is done...


Thanks for reporting it. The fix seems to be reasonably easy: self._bpoly in the roots method should be just self. And it needs a test --- the very reason the bug slipped through the pchip refactor is that it was not tested.
Would you be interested in sending a PR?

You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com//issues/6357#issuecomment-231225348, or mute the threadhttps://github.com/notifications/unsubscribe/AFl2prObc3olhnU9uWlxDfDxX930x8ycks5qTXvHgaJpZM4JHeQt.

@ev-br
Copy link
Member

ev-br commented Jul 8, 2016

No worries, we're all volunteers here :-).

Should I first clone the entire scipy repository (0.17.1 ?), then make the changes to the roots method and finally make a pull request on github?

Yes. No need to work off 0.17.1 though --- all pull requests start off the master branch. Some will get backported to maintenance branches, but there is little chance that anything will happen in 0.17.x.

For general description of the recommended workflow see e.g. http://docs.scipy.org/doc/numpy/dev/gitwash/development_workflow.html
(it's for numpy; but for scipy it's very similar.)

And don't worry about DoingThingsTheRightWay --- we all started somewhere. If you get stuck, ping this issue or a new one, or scipy-dev mailing list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected good first issue Good topic for first contributor pull requests, with a relatively straightforward solution scipy.interpolate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants