median of empty array now produces IndexError

[ethankruse]

Recently upgraded from 1.9.3 to 1.10.0 in Python 2.7 (but I get the same behavior in Python 3.5 on both Mac and Linux).

In 1.9.3, running np.median(np.array([])) produces a NaN and a warning of warnings.warn("Mean of empty slice.", RuntimeWarning).

Meanwhile, in 1.10.0, I'm now getting

IndexError: index -1 is out of bounds for axis 0 with size 0

The traceback points to the line in
python2.7/site-packages/numpy/lib/function_base.pyc in _median(a, axis, out, overwrite_input):

-> 3138         n = np.isnan(part[..., -1])

Is this intended behavior now in 1.10.0? I could switch all my code over to using np.nanmedian, which still produces the NaN result on empty arrays from 1.9.3, but this looks to be more of an accident/bug based on the tracebacks.

[lesteve]

Is this intended behavior now in 1.10.0? ... but this looks to be more of an accident/bug based on the tracebacks.

I am wondering about the exact same thing. The context if you want to know: scikit-learn tests (on master) currently fail with numpy 1.10 and this is the source of one of the errors.

[charris]

Looks like a regression to me. @empeeu @juliantaylor?

[njsmith]

@lesteve: I don't think this is anyone's fault in particular or anything, but it also sounds like beyond this issue with median there's some deeper failure of process or coordination, if it's only now after 2 months of prereleases and two actual releases (!) that anyone is running the scikit-learn test suite against numpy 1.10. Any thoughts on how we could avoid this kind of situation in the future?

Cc: @amueller @ogrisel

[amueller]

@njsmith I agree. Sorry, I've been way behind on all my open source stuff. I was a bit shocked to see it yesterday. We had a similar thing with scipy. But 1.10 was released 8 days ago, right?

It's hard to make downstream people use release candidates. I think we never get any useful feedback from ours.

It requires reading another projects mailing list and seeing if they announce a release candidate, and then actually test it ^^.

Clearly it's impossible for numpy to run downstream test suites, so this one is really on us. I don't know if there is a better mechanism than just "andy and olivier should pay more attention to the numpy and scipy mailing lists".

[argriffing]

Clearly it's impossible for numpy to run downstream test suites, so this one is really on us.

I vaguely recall something about @matthew-brett having assembled an automated testing system for a collection of downstream packages. Maybe it was this: https://github.com/MacPython/scipy-stack-osx-testing.

[amueller]

It would be cool if we could easily run the test-suites for all packages in anaconda. @teoliphant ?

[njsmith]

Anaconda is kinda a mess right now for this purpose, because (a) there's no
public numpy build recipe, (b) the way they encode numpy versions into
their platform tag means that if you do rebuild numpy then you are also
forced to rebuild everything else too, e.g. scipy (and I don't think
there's any public build recipe for scipy either). It would be awesome if
there were any easy way to get an Anaconda environment using numpy
prereleases, but it's difficult for anyone outside of Continuum to take the
next step there...
.
But I don't really like the "Andy and Olivier feel extra guilty" strategy
either. It doesn't feel very scalable :-).
.
If the problem is that no one sees the prerelease announcements, then would
some other notification mechanism work better? numpy-prelease-announce@...?
Filing a bug on scikit-learn saying "numpy 1.12b1 is released, please test
it"? (The latter could even be automated for all interested downstream
projects using a little script against the github api.)
On Oct 14, 2015 9:15 AM, "Andreas Mueller" notifications@github.com wrote:

It would be cool if we could have the test-suites for all packages in
anaconda.

—
Reply to this email directly or view it on GitHub
#6462 (comment).

[amueller]

Opening an issue on the issue tracker would be helpful, I think. It is more likely to be picked up by someone than the announcement on the numpy mailing list.

Maintaining a list of downstream packages would be an additional burden on you, but I guess it wouldn't be that bad?

[juliantaylor]

we should put into our how to release docs a step to test at least:
scipy
pandas
sklearn
skimage
astropy
(thats the set I always tested for 1.9)

those typically already catch a lot of issues and are easily available. One can even just use the debian packages which helps finding backward compatibility issues

[ogrisel]

We could have numpy master branch builds generate linux test wheels for the Ubuntu 12.04 platform and upload them to a rackspace containers so that downstream projects can use them to test their own master branch against it in their CI with an allow_failure option for that entry in their build matrix (to avoid noisy messages):

http://docs.travis-ci.com/user/customizing-the-build/#Rows-that-are-Allowed-to-Fail

[amueller]

But do we want to test against master? Or just RCs? Who would get the failures from the CI?

[ogrisel]

We could also tests against RC (we can pip install --pre numpy to get the latest beta / RC) but to avoid we would still need to automate build linux wheels to avoid rebuilding numpy (and scipy) from source in our own CI.

[charris]

Testing against the master branch would give you early notice of changes/regressions. For instance, a lot of DeprecationWarnings are going to raise errors in 1.11. Master breaks occasionally, but not often.

[charris]

I wouldn't mind tweeting releases if there are developers who follow tweets.

[shoyer]

pandas already has tests against NumPy master running Travis-CI as "allowed failures". I think I'll set this up for xray, too, and I would recommend it for scikit learn as well. Even if you're building from source NumPy only takes a few minutes to install, which is not unreasonable.

@charris Yes, there are loads of us on Twitter talking about open source software :).

[charris]

#numpy, #python ?

[ogrisel]

numpy builds quickly enough but this is not the case for scipy and ideally we would like to test scikit-learn against the master branch of both of them.

[njsmith]

Maybe someone could set up a wheelhouse on Rackspace (or wherever), and add
code to numpy, scipy, etc.'s Travis builds to upload a new wheel whenever
the tests pass?
On Oct 14, 2015 11:59 AM, "Olivier Grisel" notifications@github.com wrote:

numpy builds quickly enough but this is not the case for scipy and ideally
we would like to test scikit-learn against the master branch of both of
them.

—
Reply to this email directly or view it on GitHub
#6462 (comment).

[ogrisel]

Maybe someone could set up a wheelhouse on Rackspace (or wherever), and add code to numpy, scipy, etc.'s Travis builds to upload a new wheel whenever the tests pass?

I can do that, I have automated most of it for scikit-learn for windows on appveyor but it should for linux on travis. However we are trying to get the scikit-learn release out this week and we will do a sprint on scikit-learn next week so I am not sure I can find the time to do it in the very short term.

[ethankruse]

I'll leave the discussion of broader implications and catching things like this sooner to everyone seriously involved.

But as far as fixing this particular issue, maybe @empeeu or @juliantaylor can help as they seemed involved in this particular change over on Issue #586.

The problem occurs in line 3346 (currently) with the part[...,-1] throwing an error if part has size 0.

3341        # Check if the array contains any nan's
3342        if np.issubdtype(a.dtype, np.inexact):
3343            # warn and return nans like mean would
3344            rout = mean(part[indexer], axis=axis, out=out)
3345            part = np.rollaxis(part, axis, part.ndim)
3346            n = np.isnan(part[..., -1])

One possible simple fix is to just to skip this block if the array is of size 0 (local variable sz in the function), which effectively reverts back to the previous functionality of version 1.9.3.

3341        # Check if the array contains any nan's
3342        if np.issubdtype(a.dtype, np.inexact) and sz > 0:
3343            # warn and return nans like mean would
3344            rout = mean(part[indexer], axis=axis, out=out)
3345            part = np.rollaxis(part, axis, part.ndim)
3346            n = np.isnan(part[..., -1])

[njsmith]

Okay, summarized the conclusions from the general discussion in #6493, scipy/scipy#5379, #6494. Sorry for derailing this issue for that, but it sounds like there's some clear steps we can take to do better next time!

@ethankruse: regarding the issue with median, it sounds like you have a pretty good handle on what's needed here -- want to just submit a PR with the fix?

[matthew-brett]

It would be pretty trivial to set up travis wheel uploading to http://travis-wheels.sckicit-image.org when all the tests pass.

[njsmith]

@matthew-brett: if you want to jump in and do it before @ogrisel gets
around to it then I'm sure no one would object :-)
On Oct 16, 2015 23:40, "Matthew Brett" notifications@github.com wrote:

It would be pretty trivial to set up travis wheel uploading to
http://travis-wheels.sckicit-image.org when all the tests pass.

—
Reply to this email directly or view it on GitHub
#6462 (comment).

[ethankruse]

Ok, @njsmith, I submitted my simple fix. As far as I can tell, the median function now acts appropriately, even with empty arrays of various shapes.

I'm not used to contributing to large projects though, so I hope I did things right.

[njsmith]

Thanks Ethan! And don't worry, we all started somewhere :-)
On Oct 19, 2015 1:57 PM, "Ethan Kruse" notifications@github.com wrote:

Ok, @njsmith https://github.com/njsmith, I submitted my simple fix. As
far as I can tell, the median function now acts appropriately, even with
empty arrays of various shapes.

I'm not used to contributing to large projects though, so I hope I did
things right.

—
Reply to this email directly or view it on GitHub
#6462 (comment).

[charris]

This is fixed, but I wonder if a better targeted warning would be better.

[empeeu]

Sorry for being late to the party. @ethankruse thanks for fixing my error. Seems like this could have been avoided if there had been a unit test in place to test this scenario. When I was working on the nan update I only added tests to check the new nan behavior, and checking for empty arrays did not occur to me. Perhaps a checklist of scenarios that should be tested would have been helpful?

[njsmith]

@empeeu: Don't worry, it could have happened to anyone :-). We try to test all the cases we can think of, but there are inevitably gaps in the existing tests (many of which date back to the old days when we were less thorough about writing tests), and even when we're careful stuff will sometimes slip through. Nothing really to be done about it except to test pre-releases thoroughly, and patch up any test suite gaps when they're discovered. (Think of it this way: you've successfully discovered a bug in our test suite, and guaranteed that no-one else will ever make the same mistake again ;-).)