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

BUG: fix for issue#7835 (ma.median of 1d) #7911

Merged
merged 2 commits into from
Aug 8, 2016
Merged

BUG: fix for issue#7835 (ma.median of 1d) #7911

merged 2 commits into from
Aug 8, 2016

Conversation

skwbc
Copy link
Contributor

@skwbc skwbc commented Aug 6, 2016

fixed bugs for #7835.
I used ma.extras._median for MaskedArray, because it works properly for 1D and 0D MaskedArray.
And, I added many tests for types of outputs of ma.median.

@charris charris added this to the 1.11.2 release milestone Aug 6, 2016
@ahaldane
Copy link
Member

ahaldane commented Aug 7, 2016

Looks good to me, will merge in a bit. Thanks @skwbc !

@charris
Copy link
Member

charris commented Aug 8, 2016

@ahaldane Mind if I merge?

@ahaldane
Copy link
Member

ahaldane commented Aug 8, 2016

@charris I was just about to, I just wanted to get the backport done first

@ahaldane ahaldane merged commit 5130ef1 into numpy:master Aug 8, 2016
@ahaldane
Copy link
Member

ahaldane commented Aug 8, 2016

I went ahead with it :)

Thanks @skwbc

@charris
Copy link
Member

charris commented Aug 8, 2016

Great, thanks Allan.

@charris charris removed this from the 1.11.2 release milestone Aug 8, 2016
charris added a commit that referenced this pull request Aug 8, 2016
Backport #7911: BUG: fix for issue#7835 (ma.median of 1d)
return masked_array(np.median(getdata(a, subok=True), axis=axis,
out=out, overwrite_input=overwrite_input,
keepdims=keepdims), copy=False)
if not hasattr(a, 'mask'):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was the count_nonzero removed?
its performance impact is practically zero but the gain on empty mask is huge

Copy link
Contributor Author

@skwbc skwbc Aug 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to have simplified the if-condition (that is, MaskedArray or others), and I just didn't know that np.median is much faster than np.ma.extras._median.
So, we should add or np.count_nonzero(a.mask) == 0 here.
Should I make pull request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants