8000 BUG: Fixes ndarray.fill to accept maximum uint64 value by johntyree · Pull Request #5618 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG: Fixes ndarray.fill to accept maximum uint64 value #5618

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
Mar 1, 2015
Merged

BUG: Fixes ndarray.fill to accept maximum uint64 value #5618

merged 1 commit into from
Mar 1, 2015

Conversation

johntyree
Copy link
Contributor

This PR changes np.ndarray.fill to attempt to convert to uint64 if its argument is a Python integer and conversion toint64 fails.

np.ndarray.fill now accepts the Python long 2**64 - 1 as a value. Previously it failed because the value overflows an int64 and uint64 was not attempted. Constructing the value as a uint64 manually and then passing it to fill succeeded.

Fixes #5612

johntyree
johntyree reviewed Feb 28, 2015
View reviewed changes
numpy/core/src/multiarray/convert.c
npy_longlong v = PyLong_AsLongLong(obj);
if (v == -1 && PyErr_Occurred()) {
return -1;
/* Long long failed, try unsigned long long */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not particularly happy with the nesting here, but this case is slightly different from the others in that we're mapping from a single Python type to one of several NumPy types. Suggestions welcome.

@juliantaylor
Copy link
Contributor

looks good thanks
can you still squash the commits into one with git rebase -i master
as its probably still appropriate for 1.9.2 please also run git rebase --onto $(git merge-base master origin/maintenance/1.9.x) HEAD^ assuming origin is the remote name of the main numpy repo.

@johntyree johntyree force-pushed the arrfill_ulonglong branch from f8f5d96 to 11dfe93 Compare March 1, 2015 00:14
@johntyree
Copy link
Contributor Author

So I think I understood you correctly. There is now a single commit based off of 50afb0f.

@juliantaylor
Copy link
Contributor

perfect, that allows us to easily merge it into both branches.
thanks

juliantaylor added a commit that referenced this pull request Mar 1, 2015
@juliantaylor
Merge pull request #5618 from johntyree/arrfill_ulonglong
a29f50e 
BUG: Fixes ndarray.fill to accept maximum uint64 value
@juliantaylor juliantaylor merged commit a29f50e into numpy:master Mar 1, 2015
juliantaylor added a commit that referenced this pull request Mar 1, 2015
@juliantaylor
Merge pull request #5618 from johntyree/arrfill_ulonglong
bf6f80d 
BUG: Fixes ndarray.fill to accept maximum uint64 value
@johntyree
Copy link
Contributor Author

\o/ Thanks

7D61

@johntyree johntyree deleted the arrfill_ulonglong branch March 1, 2015 01:14
@njsmith njsmith mentioned this pull request Sep 24, 2015
Closed
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.

Python long raises OverflowError while same value as np.uint64 does not.
2 participants
@johntyree @juliantaylor
0