8000 BUG: preserve ndarray subclasses in median by juliantaylor · Pull Request #3851 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG: preserve ndarray subclasses in median#3851

Merged
njsmith merged 1 commit intonumpy:masterfrom
juliantaylor:median-subclass
Oct 2, 2013
Merged

BUG: preserve ndarray subclasses in median#3851
njsmith merged 1 commit intonumpy:masterfrom
juliantaylor:median-subclass

Conversation

@juliantaylor
Copy link
Contributor
@juliantaylor juliantaylor commented Oct 1, 2013

closes gh-3846

@juliantaylor
Copy link
Contributor Author
juliantaylor commented Oct 1, 2013

the same should probably be done for percentile

@njsmith
Copy link
Member
njsmith commented Oct 1, 2013

Earlier I somehow had the idea that there was a ndarray.median method. Now
that I see I was wrong and there isn't, I'm -1 on checking for a .median
method on random objects. The .mean thing is ridiculous, but it's
ridiculous that we already have to deal with. Adding random special method
fallbacks on a whim is just going to leave us with a messier hairball to
clean up later.
On 1 Oct 2013 19:53, "Julian Taylor" notifications@github.com wrote:

the same should probably be done for percentile

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3851#issuecomment-25477216
.

@rgommers
Copy link
Member
rgommers commented Oct 1, 2013

Agree with @njsmith.

@charris
Copy link
Member
charris commented Oct 2, 2013

Also agree with @njsmith.

@charris
Copy link
Member
charris commented Oct 2, 2013

@astrofrog Do we need this patch for backwards compatibility or does the mean hack serve that purpose? I'm a bit lost as to the situation.

@astrofrog
Copy link
Contributor
astrofrog commented Oct 2, 2013

@charris - as long as .mean gets called on a subclass if present, then it should be fine. If you update this pull request to remove the checking of .median (since that is the consensus) I can test out the rest of the pull request.

@njsmith
Copy link
Member
njsmith commented Oct 2, 2013

IIUC the only part that's needed for astropy is the replacement of
'asarray' with 'asanyarray'.

On Wed, Oct 2, 2013 at 3:21 PM, Thomas Robitaille
notifications@github.comwrote:

@charris https://github.com/charris - as long as .mean gets called on a
subclass if present, then it should be fine. If you update this pull
request to remove the checking of .median (since that is the consensus) I
can test out the rest of the pull request.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3851#issuecomment-25542247
.

@juliantaylor
Copy link
Contributor Author
juliantaylor commented Oct 2, 2013

updated to only add asanyarray

@charris
Copy link
Member
charris commented Oct 2, 2013

@astrofrog Can you check this out?

@astrofrog
Copy link
Contributor
astrofrog commented Oct 2, 2013

@charris @juliantaylor - I can confirm this fixes the errors we were seeing in Astropy - thanks!

@njsmith
Copy link
Member
njsmith commented Oct 2, 2013

Great, ready for backport then.

njsmith added a commit that referenced this pull request Oct 2, 2013
@njsmith
Merge pull request #3851 from juliantaylor/median-subclass
f665c61 
BUG: preserve ndarray subclasses in median
@njsmith njsmith merged commit f665c61 into numpy:master Oct 2, 2013
@charris charris mentioned this pull request Oct 2, 2013
Merged
@juliantaylor juliantaylor mentioned this pull request Oct 16, 2013
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

np.median behavior broken for array subclasses in 1.8.0rc1

5 participants

@juliantaylor @njsmith @rgommers @charris @astrofrog
0