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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 2, 2013
Merged

BUG: preserve ndarray subclasses in median #3851

merged 1 commit into from
Oct 2, 2013

Conversation

juliantaylor
Copy link
Contributor

closes gh-3846

@juliantaylor
Copy link
Contributor Author

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

@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

updated to only add asanyarray

@charris
Copy link
Member
charris commented Oct 2, 2013

@astrofrog Can you check this out?

@astrofrog
Copy link
Contributor

@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< 86FA /a> 
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