BUG: np.sum with ``keepdims=True`` fails on masked_array by astrofrog · Pull Request #7312 · numpy/numpy

astrofrog · 2016-02-22T22:42:19Z

When calling np.sum on a masked array, and using the keepdims option, an error is raised:

In [1]: import numpy as np 

In [2]: np.sum(np.ma.masked_array([1,2,3],mask=[0,0,1]),keepdims=True)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-8-482ed005c74d> in <module>()
----> 1 np.sum(np.ma.masked_array([1,2,3],mask=[0,0,1]),keepdims=True)

/Users/tom/miniconda3/envs/production27/lib/python2.7/site-packages/numpy-1.12.0.dev0+0dbcd2f-py2.7-macosx-10.5-x86_64.egg/numpy/core/fromnumeric.pyc in sum(a, axis, dtype, out, keepdims)
   1843             pass
   1844         else:
-> 1845             return sum(axis=axis, dtype=dtype, out=out, **kwargs)
   1846     return _methods._sum(a, axis=axis, dtype=dtype,
   1847                          out=out, **kwargs)

TypeError: sum() got an unexpected keyword argument 'keepdims'

astrofrog · 2016-02-22T22:27:37Z

This can be triggered even without the user passing keepdims:

In [6]: x = np.ma.masked_array([1,2,np.nan],mask=[0,0,1])

In [7]: np.nanvar(x)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-7-ede4d9457a87> in <module>()
----> 1 np.nanvar(x)

/Users/tom/miniconda3/envs/production27/lib/python2.7/site-packages/numpy-1.12.0.dev0+0dbcd2f-py2.7-macosx-10.5-x86_64.egg/numpy/lib/nanfunctions.py in nanvar(a, axis, dtype, out, ddof, keepdims)
   1182         # the reason that it drops the keepdims kwarg is to force keepdims=True
   1183         # so this used to work by serendipity.
-> 1184         cnt = np.sum(~mask, axis=axis, dtype=np.intp, keepdims=_keepdims)
   1185         avg = np.sum(arr, axis=axis, dtype=dtype, keepdims=_keepdims)
   1186         avg = _divide_by_count(avg, cnt)

/Users/tom/miniconda3/envs/production27/lib/python2.7/site-packages/numpy-1.12.0.dev0+0dbcd2f-py2.7-macosx-10.5-x86_64.egg/numpy/core/fromnumeric.pyc in sum(a, axis, dtype, out, keepdims)
   1841         print(type(a))
   1842         sum = a.sum
-> 1843         return sum(axis=axis, dtype=dtype, out=out, **kwargs)
   1844     return _methods._sum(a, axis=axis, dtype=dtype,
   1845                          out=out, **kwargs)

TypeError: sum() got an unexpected keyword argument 'keepdims'

astrofrog · 2016-02-22T22:31:51Z

I'm working on a fix

astrofrog · 2016-02-22T22:42:38Z

I've attached a fix!

njsmith · 2016-02-22T22:52:30Z

Lots of PEP8 violations in this function (spaces after commas) -- could you fix that? Otherwise LGTM

Oops, thanks for pointing that out - should be fixed now

…eepdims= keyword argument, and include regression test

ahaldane · 2016-02-22T23:07:28Z

Before you work too hard on this, may I suggest looking at my PR #5706, which adds keepdims, and is finished but just waiting for a review.

After a huge discussion, we concluded you can't directly add a keepdims arg, instead you have to catch it through **kwargs, because of the case of masked arrays of ndarray subclasses (eg masked matrices), which want to change the default value of keepdims.

njsmith · 2016-02-23T01:10:08Z

@ahaldane: oh good point

astrofrog · 2016-02-23T08:26:43Z

@njsmith @ahaldane - I'm fine with closing this, as long as the equivalent fix in #5706 is included in 1.11.x - will that be the case?

astrofrog · 2016-02-23T08:28:10Z

Just to clarify, this is causing test failures in Astropy, so it would be good to have this fix included in 1.11.0 final - thanks!

njsmith · 2016-02-23T08:35:22Z

@astrofrog: wait, this is a regression? Can you give an example of code that used to work but now doesn't, and which version it worked in?

astrofrog · 2016-02-23T08:51:55Z

@njsmith - it is a regression but actually after double checking I think it doesn't affect 1.11.0 pre-releases.

This works in Numpy 1.9, 1.10, and 1.11.0rc1 (using the git tag):

>>> import numpy as np
>>> x = np.ma.masked_array([1,2,np.nan],mask=[0,0,1])
>>> np.nanvar(x)
0.25

but fails in master:

>>> import numpy as np

>>> x = np.ma.masked_array([1,2,np.nan],mask=[0,0,1])

>>> np.nanvar(x)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-3-ede4d9457a87> in <module>()
----> 1 np.nanvar(x)

/Users/tom/miniconda3/envs/production35/lib/python3.5/site-packages/numpy-1.12.0.dev0+da2ad32-py3.5-macosx-10.5-x86_64.egg/numpy/lib/nanfunctions.py in nanvar(a, axis, dtype, out, ddof, keepdims)
   1178         # the reason that it drops the keepdims kwarg is to force keepdims=True
   1179         # so this used to work by serendipity.
-> 1180         cnt = np.sum(~mask, axis=axis, dtype=np.intp, keepdims=_keepdims)
   1181         avg = np.sum(arr, axis=axis, dtype=dtype, keepdims=_keepdims)
   1182         avg = _divide_by_count(avg, cnt)

/Users/tom/miniconda3/envs/production35/lib/python3.5/site-packages/numpy-1.12.0.dev0+da2ad32-py3.5-macosx-10.5-x86_64.egg/numpy/core/fromnumeric.py in sum(a, axis, dtype, out, keepdims)
   1843             pass
   1844         else:
-> 1845             return sum(axis=axis, dtype=dtype, out=out, **kwargs)
   1846     return _methods._sum(a, axis=axis, dtype=dtype,
   1847                          out=out, **kwargs)

TypeError: sum() got an unexpected keyword argument 'keepdims'

So maybe we are ok for the 1.11.0 release

seberg · 2016-02-23T08:57:52Z

We recently put in a PR fixing keepdims propagation to subclasses for things like sum. That probably is the reason. Which means that nanvar might be broken for weird reduction axis though.

charris · 2016-03-07T01:17:33Z

@seberg That would probably be #4619. Looks like we need to carefully review that again together with this.

seberg · 2016-03-07T09:37:35Z

Ohh, yes, I never looked at this a lot. The issue here is not so much the lack of keepdims in MA as much as the use of keepdims in nanvar, etc.
Not sure whether it is a big issue in practice, but it would be nice to extend the use of _NoValue to those functions like nanvar, etc. where possible at least.

seberg · 2016-03-07T09:37:55Z

Tagging with 1.12 milestone, just so we revisit it before a release.

homu · 2016-04-04T21:08:31Z

☔ The latest upstream changes (presumably #5706) made this pull request unmergeable. Please resolve the merge conflicts.

seberg · 2016-04-04T21:13:29Z

@ahaldane was this fixed?

ahaldane · 2016-04-04T21:23:03Z

Yeah this is fixed. Closing.

astrofrog changed the title ~~np.sum with keepdims=True fails on masked_array~~ BUG: np.sum with keepdims=True fails on masked_array Feb 22, 2016

njsmith reviewed Feb 22, 2016
View reviewed changes

astrofrog mentioned this pull request Feb 22, 2016

Be more specific regarding which exception to catch in data_utils.py astropy/astropy#4624

Closed

BUG: fixed a bug that was caused by masked_array.sum not having the k…

644865f

…eepdims= keyword argument, and include regression test

charris added 00 - Bug component: numpy.ma masked arrays labels Feb 22, 2016

astrofrog force-pushed the missing-ma-sum-keepdims branch from 8fbf36e to 644865f Compare February 22, 2016 23:06

njsmith mentioned this pull request Feb 23, 2016

ENH: make some masked array methods behave more like ndarray methods #5706

Merged

seberg added this to the 1.12.0 release milestone Mar 7, 2016

mhvk mentioned this pull request Mar 9, 2016

BUG: incorrect type for objects whose __len__ fails #7397

Merged

mhvk mentioned this pull request Mar 23, 2016

Ensure we don't use floats for number of elements in linspace. astropy/astropy#4722

Merged

ahaldane closed this Apr 4, 2016

Uh oh!

Uh oh!

Conversation

astrofrog commented Feb 22, 2016

Uh oh!

astrofrog commented Feb 22, 2016

Uh oh!

astrofrog commented Feb 22, 2016

Uh oh!

astrofrog commented Feb 22, 2016

Uh oh!

njsmith Feb 22, 2016

Choose a reason for hiding this comment

Uh oh!

astrofrog Feb 22, 2016

Choose a reason for hiding this comment

Uh oh!

ahaldane commented Feb 22, 2016

Uh oh!

njsmith commented Feb 23, 2016

Uh oh!

astrofrog commented Feb 23, 2016

Uh oh!

astrofrog commented Feb 23, 2016

Uh oh!

njsmith commented Feb 23, 2016

Uh oh!

astrofrog commented Feb 23, 2016

Uh oh!

seberg commented Feb 23, 2016

Uh oh!

charris commented Mar 7, 2016

Uh oh!

seberg commented Mar 7, 2016

Uh oh!

seberg commented Mar 7, 2016

Uh oh!

homu commented Apr 4, 2016

Uh oh!

seberg commented Apr 4, 2016

Uh oh!

ahaldane commented Apr 4, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants