Fix incorrect output shape of UnivariateSpline #197

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@jpaalasm
Contributor

The shape of the output of UnivariateSpline.call does not match the shape of the input when the input is a one-item array. The behavior is unexpected and leads to bugs.

In [1]: import scipy.interpolate
In [2]: f = scipy.interpolate.UnivariateSpline([1,2,3,4], [1,2,3,4])

In [3]: f(1)
Out[3]: 0.99999999999999956

In [4]: f([1])
Out[4]: 0.9999999999999995

In [5]: f([1, 2])
Out[5]: array([ 1.,  2.])

The consistent bahavior would be f([1]) -> [1.0] rather than the current f([1]) -> 1.0

See http://projects.scipy.org/scipy/ticket/1534

@rgommers
Member

The new keyword is not going to work well - when you want to remove it again (which you will), you still have to break backwards compatibility. I'd say it is much better to just make the change and possibly let a minor amount of code break. It's not like this will give different results (which would be very bad), it can only make some code fall over.

@rgommers
Member

I'm +0.5 on the behavior change; it does make things more consistent.

@jpaalasm
Contributor

I agree. I think it's better to just make the change (to have splev always return y) instead of playing with a new_behavior parameter.

@rgommers
Member

OK, so I guess the best way forward here would be for you to adjust your patch to simply return y, then propose this on the mailing list.

@jpaalasm jpaalasm ENH: Always return a sequence from splev
The old behavior was to return a scalar when the length of input
is one. That is unexpected behavior.
883105b
@jpaalasm
Contributor

I changed the patch to be such that splev just does "return y" in all cases.

It seems that the weird behavior of splev is not even handled in the tests, because the tests pass after this change.

@pv
Member
pv commented Apr 25, 2012

+1 for this change. It's probably safer to go from array scalars to 1-d arrays than the other way around. We'll need to mention this change in the release notes, of course.

@jpaalasm
Contributor

Another oddity with splev is that the ext-parameter is not passed to the inner call of splev if "parametric mode" is used.

t,c,k = tck
try:
    c[0][0]
    parametric = True
except:
    parametric = False
if parametric:
    return map(lambda c, x=x, t=t, k=k, der=der : splev(x, [t,c,k], der), c)
@rgommers
Member
rgommers commented May 5, 2012

ext not being passed through looks like a mistake to me.

@pv
Member
pv commented May 19, 2012

Fixing it this way makes several tests to fail. Here's a fixed attempt: https://github.com/pv/scipy-work/tree/splev-fix

@rgommers
Member

@pv: I'm not a fan of returning array scalars. They're confusing and may still require special-casing. Do you see a benefit over returning a 1-D array with 1 element?

@pv
Member
pv commented May 19, 2012

@rgommers: well, some tests fail if we return 1-D arrays always. Moreover, also interp1d preserves output.shape==input.shape like this.

@rgommers
Member

You could also decide to change the tests (seems straightforward) or still return a scalar. It's not that important, but imho arrays that can't be indexed are more confusing than helpful.

Something else: myasarray should be removed completely from that file.

@pv
Member
pv commented May 19, 2012

I'd weigh input.shape == output.shape as more important than possible issues with the return values... Anyway, my point with the tests was that maybe something else also fails in third-party code. I didn't look in depth in the failures, though, to see how serious they are.

@rgommers
Member

OK, I'd say push your fix then.

@rgommers
Member
rgommers commented Jun 2, 2012

@pv: can you push your fix before Wednesday?

@rgommers
Member
rgommers commented Jun 9, 2012

@pv, I took the liberty of merging your commits in 2ad7d96.

@rgommers rgommers closed this Jun 9, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment