Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

np.mean with keepdims=True does not work on memmap #6750

Closed
samuelstjean opened this issue Dec 1, 2015 · 8 comments
Closed

np.mean with keepdims=True does not work on memmap #6750

samuelstjean opened this issue Dec 1, 2015 · 8 comments

Comments

@samuelstjean
Copy link
Contributor

I'm using numpy 1.9.2 (built locally with pip and linked to openblas) and there seems to be a strange bug with memmap. I checked through recent issues/pr and nothing seemed reported (I don't feel like building the master to check this out as I tend to stay on the same version of everything to compare results between themselves). Anyway

In [95]: data.shape
Out[95]: (145, 174, 145, 288)

In [101]: np.mean(data[..., 0:10], axis=-1, keepdims=True).shape
Out[101]: (145, 174, 145)

In [105]: type(data)
Out[105]: numpy.core.memmap.memmap

In [106]: data = np.array(data)

In [107]: np.mean(data[..., 0:10], axis=-1, keepdims=True).shape
Out[107]: (145, 174, 145, 1)

So the memmap doesn't play well with the keepdims=True, but the numpy array version works as expected.

@seberg
Copy link
Member

seberg commented Dec 1, 2015

Old story, though I did not find a duplicate, so....

If you look at the code for mean or many other of these types of functions, they do not attempt to pass on "keepdims" while attempting to call the method at all. I frankly thought we fixed it....
The only real fix I see is to use try to pass in keepdims if and only if it is passed in. unfortunately. I think that is not 100% clear without some thought how to do it, but probably a good solution can be found.

The code:

    if type(a) is not mu.ndarray:
        try:
            mean = a.mean
            return mean(axis=axis, dtype=dtype, out=out)
        except AttributeError:
            pass

    return _methods._mean(a, axis=axis, dtype=dtype,
                          out=out, keepdims=keepdims)

@samuelstjean
Copy link
Contributor Author

Well, at least that explains why my local test for some function was not working on some cases, I'll just use a dirty workaround for the time being, thanks. I does confirm I'm not doing weird stuff at least and it's an unintended side effect.

@mhvk
Copy link
Contributor

mhvk commented Dec 2, 2015

@seberg - I guess one has two choices:

  1. Make the keepdims default None, and pass anything else on for the non-ndarray method, and change None to False for the call to _methods._mean.
  2. Add a generic **kwargs, removing keepdims from the explicit keywords, and pass **kwargs in both cases.
    Given that we probably don't really want to change the function signature, option 1 would seem best.

@seberg
Copy link
Member

seberg commented Dec 2, 2015

Yes, 1. is best, unless you want to be able to pass None into the subclass for some reason. Which I agree is probably something we don't need. But I am pretty sure there has been a discussion abotu this before.

@njsmith
Copy link
Member

njsmith commented Dec 2, 2015

**kwargs is more technically correct I guess (it avoids adding special
semantics to an explicitly passed keepdims=None and getting in a situation
where annoying end-user code starts passing it explicitly). It's clear
enough what's to be done though I think, it's just slightly annoying to
implement :-).
On Dec 2, 2015 7:12 AM, "seberg" notifications@github.com wrote:

Yes, 1. is best, unless you want to be able to pass None into the
subclass for some reason. Which I agree is probably something we don't
need. But I am pretty sure there has been a discussion abotu this before.


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

@ahaldane
Copy link
Member

ahaldane commented Dec 2, 2015

The lack of support of keepdims in np methods came up before, see http://thread.gmane.org/gmane.comp.python.numeric.general/60501

The problem is that many ndarray subclasses do not support the keepdims arg, so we have to be careful when trying to pass it on. @njsmith's reply there made sense to me.

@samuelstjean
Copy link
Contributor Author

I think that this super old thing is fixed now though and can probably be closed.

@seberg
Copy link
Member

seberg commented Mar 4, 2019

Yes, thanks for the note @samuelstjean.

@seberg seberg closed this as completed Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants