convolve.py: use arr[tuple(seq)] instead of arr[seq] -- with numpy 1.14.0

[NimSed]

Hi.
I get such warnings when using numpy 1.14.0:

/home/nima/.local/lib/python3.5/site-packages/astropy/convolution/convolve.py:692: FutureWarning: Using a non-tuple sequence for multidimensional indexing is deprecated; use `arr[tuple(seq)]` instead of `arr[seq]`. In the future this will be interpreted as an array index, `arr[np.array(seq)]`, which will result either in an error or a different result.
  bigimwt[arrayslices] = 1.0 - nanmaskarray * interpolate_nan
/home/nima/.local/lib/python3.5/site-packages/astropy/convolution/convolve.py:700: FutureWarning: Using a non-tuple sequence for multidimensional indexing is deprecated; use `arr[tuple(seq)]` instead of `arr[seq]`. In the future this will be interpreted as an array index, `arr[np.array(seq)]`, which will result either in an error or a different result.
  bigimwt[arrayslices] = wtsm.real[arrayslices]
/home/nima/.local/lib/python3.5/site-packages/astropy/convolution/convolve.py:735: FutureWarning: Using a non-tuple sequence for multidimensional indexing is deprecated; use `arr[tuple(seq)]` instead of `arr[seq]`. In the future this will be interpreted as an array index, `arr[np.array(seq)]`, which will result either in an error or a different result.
  result = rifft[arrayslices].real

Rolling back to numpy 1.10 seems to solve the problem.

[pllim]

Thanks for reporting this, @NimSed ! Could you please provide a minimal example to reproduce the warning?

Looks like this might still be a problem in master.

astropy/astropy/convolution/convolve.py

Line 709 in 1f34264

bigimwt[arrayslices] = 1.0 - nanmaskarray * interpolate_nan

I wonder why CI did not catch this... @adamginsburg ?

[mhvk]

This is a recent change in numpy, with the intent to make indexing with a list always return the items in that list, i.e., that only with a tuple you get indexes for each axis. One can add tuple(arrayslices) or perhaps ensure arrayslices is a tuple in the first place.

[bsipocz]

I wonder why CI did not catch this...

I've seen this on travis a few days ago, but didn't get to the point to pin down why an error wasn't raised, so the draft issue was never opened.

[keflavich]

I've recently had to make the same change in other projects. tuple(whatever was previously a list) is the right approach, as @mhvk suggested.

[astrofrog]

I'm actually surprised at the number of backward-incompatible changes Numpy is making these days!

[mhvk]

I'm actually surprised at the number of backward-incompatible changes Numpy is making these days!

There has been quite a bit of discussion on that for each case, but, yes, it is true, mostly because there are now a couple of developers who have started to go through and clean up... The most painful, oddly, was the work to make representations of floats consistent, since that breaks so many doctests.

Anyway, it is not THAT bad - the number of if NUMPY_LT_* statements has decreased very substantially.

[astrofrog]

@mhvk - I'm just jealous given that I fancy doing that for some parts of astropy ;)

[bsipocz]

I'm just jealous given that I fancy doing that for some parts of astropy ;)

May be that's totally doable for 4.0? ;)

[astrofrog]

Maybe a topic for the coordination meeting - under what conditions do we envisage ever breaking backward compatibility in future? The reason Python 3 was bad is because they packaged all the breaks in a single release. In a way, Numpy's approach is less impactful - just break one thing per release ;)

[bsipocz]

Yes, numpy is working their way on the same approach as setuptools, sphinx and conda where one awaits the new release with a lump in the stomach to see what's broken this time. Not sure which one is better, from the users' point of view, this is more invisible than having all the change at once I guess.

[pllim]

just break one thing per release

I dunno. I have mixed feeling on both from a downstream maintainer's point of view. 🙈

[mhvk]

Long live our numpy-dev travis test runs! 🎆

[Cadair]

Some times you just gotta break stuff 😁

[bsipocz]

There are still a few of this very same warning turn up in time, and one also comes from scipy, but all the ones from convolution is cleared up, so I go ahead and close this issue (the rest can and should be addressed along with the rest of the warnings we have).