Skip to content

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

Merged
ahaldane merged 2 commits into
numpy:masterfrom
skwbc:fix_issue#7835
Aug 8, 2016
Merged

ahaldane merged 2 commits into
numpy:masterfrom
skwbc:fix_issue#7835

Conversation

@skwbc

@skwbc skwbc commented Aug 6, 2016

Copy link
Copy Markdown
Contributor

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

ahaldane commented Aug 7, 2016

Copy link
Copy Markdown
Member

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

@charris

charris commented Aug 8, 2016

Copy link
Copy Markdown
Member

@ahaldane Mind if I merge?

@ahaldane

ahaldane commented Aug 8, 2016

Copy link
Copy Markdown
Member

@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

ahaldane commented Aug 8, 2016

Copy link
Copy Markdown
Member

I went ahead with it :)

Thanks @skwbc

@charris

charris commented Aug 8, 2016

Copy link
Copy Markdown
Member

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)
Comment thread numpy/ma/extras.py
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
Copy Markdown
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

@skwbc skwbc Aug 20, 2016 •

Copy link
Copy Markdown
Contributor Author

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