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

ENH: allow non-monotonic input in interp1d #2946

Closed
wants to merge 5 commits into from

Conversation

djarecka
Copy link
Contributor

I added sorting to interpolate.interp1d, so the values of x array might be in any order. I also added a new keyword argument assume_sorted=False, if assume_sorted==True, the sorting is omitted. Not sure if the way of sorting is the best one.
I added a unit test, that checks if the behavior for unsorted arrays is the same as for sorted ones.

These are my first changes to the code, so please let me know if something should be done in a different way/place etc.

…ht be in any order. I also added a unit test checking the behavior for unsorted arrays
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling e732d32 on djarecka:fix-interp1d into ce9d9d1 on scipy:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0bb339a on djarecka:fix-interp1d into a3e9c7f on scipy:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0bb339a on djarecka:fix-interp1d into a3e9c7f on scipy:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 981a0e1 on djarecka:fix-interp1d into a3e9c7f on scipy:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 981a0e1 on djarecka:fix-interp1d into a3e9c7f on scipy:master.

@@ -268,7 +268,7 @@ def __call__(self, x, y, dx=0, dy=0):
class interp1d(_Interpolator1D):
"""
interp1d(x, y, kind='linear', axis=-1, copy=True, bounds_error=True,
fill_value=np.nan)
assume_sorted=False, fill_value=np.nan)
Copy link
Member

Choose a reason for hiding this comment

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

New keyword arguments should be added last, so that the function remains backward-compatible

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling aa5141b on djarecka:fix-interp1d into a3e9c7f on scipy:master.

@@ -349,6 +354,11 @@ def __init__(self, x, y, kind='linear', axis=-1,
x = array(x, copy=self.copy)
y = array(y, copy=self.copy)

if not assume_sorted:
ind = np.argsort(x)
x = x[ind]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just x.sort()? argsort builds an array of intp that is typically as large as x (depending on the platform, but on a 64-bit box with dtype np.float32, it's even twice as large as x).

Copy link
Member

Choose a reason for hiding this comment

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

The correspondence between x and y needs to be maintained in the sort.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, right! Sorry, missed that.

@larsmans
Copy link
Contributor

larsmans commented Oct 1, 2013

Relates to #2283.

@djarecka
Copy link
Contributor Author

djarecka commented Oct 1, 2013

I just didn't know how to sort the y array by the x array in a nice way without creating ind array. I know that sometimes you can use numpy.rec.fromarrays(x,y), but I believe it works only if x and y have the same shape.

rgommers added a commit that referenced this pull request Feb 4, 2014
@rgommers
Copy link
Member

rgommers commented Feb 4, 2014

@djarecka using the ind arrays is a good way to do this.

@rgommers
Copy link
Member

rgommers commented Feb 4, 2014

OK I merged this PR in 5309706 after doing the following:

Oh yes, and edited the PR title.

@rgommers rgommers closed this Feb 4, 2014
@rgommers
Copy link
Member

rgommers commented Feb 4, 2014

Thanks Dorota, and sorry it took so long to get around to merging this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or improvement scipy.interpolate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants