Update scipy dependency to v0.13.2 #934
Conversation
| @@ -874,6 +874,32 @@ def __init__(self, interpolator): | |||
| .. note:: These are stored with the interpolator.axis last. | |||
|
|
|||
| """ | |||
| # Roll interpolator.axis to the end if scipy no longer does it for us. | |||
| if not self._interp1d_rolls_y(): | |||
There was a problem hiding this comment.
How about we make this check once instead of on every use?
There was a problem hiding this comment.
Still need to address this issue @dkillick.
You could perform a module level test and refer to the global result to determine whether to roll or not ...
There was a problem hiding this comment.
Mm, but I was under the impression that setting module-level variables was not best practice? Also, surely the processing cost in repeating this test is not great?
There was a problem hiding this comment.
Okay, I'm with you on this one @dkillick. We'll leave it as is. If anyone is mortally offended then its an isolated patch to change.
There was a problem hiding this comment.
I've no real aversion to a "private" module variable. Or you could have a class attribute computed by a private module function.
ghost
assigned 
| @@ -874,6 +874,32 @@ def __init__(self, interpolator): | |||
| .. note:: These are stored with the interpolator.axis last. | |||
|
|
|||
| """ | |||
| # Roll interpolator.axis to the end if scipy no longer does it for us. | |||
| if not self._interp1d_rolls_y(): | |||
| self.y = np.rollaxis(self.y, self._interpolator.axis, self.y.ndim) | |||
There was a problem hiding this comment.
Isn't this just self.y = self.y.T ... or am I misunderstanding?
There was a problem hiding this comment.
For a 2-dimensional case yes! But it may not always be 2D...
There was a problem hiding this comment.
Ahhh, okay ... so that would mean that you've got supporting tests to show that it's behaving for non-2D situations 😉 ...
There was a problem hiding this comment.
So there is test_interpolation.TestLinearExtrapolator.test_simple_3d_axis1 that uses interp1d on a (3, 4, 2) array and extrapolates along axis=1 and does not fail...
|
@dkillick you've got a bunch of test failures with [email protected] ... is that expected? |
|
@bjlittle that's odd, all my tests are fine, and Travis says all is well too. Have you fetched upstream? This branch does rely on other merges, notably those from branches from @rhattersley |
|
My fault @dkillick, I was pointing to a different Cartopy version, phew! |
|
👍 Working fine for NumPy v1.7.2 and SciPy 0.13.2 ... just need to address one last issue and then we're good to merge @dkillick! |
|
@dkillick could you please squash your commit history, then I'll merge, ta! |
|
Squish |
|
See DPeterK#4 ... |
|
Just a reminder for this to close #493 when merged. |
|
Do you wanna squish again @dkillick ? ... and thanks @ajdawson! |
|
Re-squished. |
|
@esc24 has requested a couple of minor docstring changes so I'm going to go ahead and make them and then hopefully push the final commit to this PR! |
|
Docstring changes made. |
|
Seem to have a Travis Pandas issue ... I'm investigating |
|
Travis is now using the latest pandas v0.13.0 which is causing the test failures. |
|
When did that start happening? |
|
@esc24 says he is literally about to jump on fixing this! |
|
Okay, after investigating I'm happy that this PR isn't the cause of the Tavis test failures, so we're good to go @dkillick 👍 , thanks! |
Update scipy dependency to v0.13.2
|
Woop 😄 Thanks @bjlittle! |
This updates the scipy dependency in Iris to v0.13.2, with backward compatibility to v0.11.0 maintained.
Notable issues encountered in updating this dependency were changes in array ordering, usually from 'F' to 'C' ordering and a change in the behaviour of
scipy.interpolate.interpolate.interp1d.Some CML changes made on account of the array ordering differences.
The changes to
interp1dremove previous incorrect behaviour that changed the ordering of theyarray internally to the function. Asiris.analysis.interpolate.Linear1dExtrapolatorwas written based on this incorrect behaviour, to maintain backward compatibility and make minimal code changes, this behaviour is tested for, with theyarray's order changed as necessary.Closes #493