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 mean for float 16 non-array inputs #8524

Merged
merged 1 commit into from Jan 24, 2017

Conversation

juliantaylor
Copy link
Contributor

No description provided.

Copy link
Contributor

@f0k f0k left a comment

Choose a reason for hiding this comment

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

That was fast! I'd suggest a minimal addition to the test.

@@ -4380,6 +4380,12 @@ def setUp(self):
self.omat = np.array([Decimal(repr(r)) for r in self.rmat.flat])
self.omat = self.omat.reshape(4, 5)

def test_python_type(self):
for x in (np.float16(1.), 1, 1., 1+0j):
Copy link
Contributor

@f0k f0k Jan 24, 2017

Choose a reason for hiding this comment

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

To have a test that triggers the bug fixed in this PR, you may want to add [np.float16(1.)] to this tuple (i.e., a float16 wrapped in a list). The others don't run into the is_float16_result and not hasattr(a, "dtype") case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all tests are scalars wrapped into a list, though just scalars might be useful too

Copy link
Contributor

Choose a reason for hiding this comment

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

all tests are scalars wrapped into a list

Ooops, right. Great then! Good to merge from my perspective. Thank you for taking this!

Copy link
Contributor

@f0k f0k left a comment

Choose a reason for hiding this comment

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

Seems I cannot edit my previous review from "Request changes" to "Approve". Not that it matters too much, since I'm just a contributor without write access. Anyway, this is my new review which approves the PR :)

@charris charris merged commit f737b35 into numpy:master Jan 24, 2017
@charris
Copy link
Member

charris commented Jan 24, 2017

Thanks Julian. Should I do the backport?

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.

None yet

3 participants