Skip to content

BUG: Broadcast Arguments in Random Integer Generation #6902

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

Closed
wants to merge 1 commit into from
Closed

BUG: Broadcast Arguments in Random Integer Generation #6902

wants to merge 1 commit into from

Conversation

gfyoung
Copy link
Contributor

@gfyoung gfyoung commented Dec 30, 2015

Title is self-explanatory.

@gfyoung
Copy link
Contributor Author

gfyoung commented Dec 30, 2015

Anyone have ideas about how I might add functionality for the following call?

>>> import numpy as np
>>> np.random.randint(5, [10, None])

It makes sense to allow this call given that we allow high to be None currently. In my PR, it doesn't work because numpy will throw a type error when trying to convert the None element to long. However, I can't think of anything "fast" / segfault-safe that isn't iterating through both the low and high arrays.

@njsmith
Copy link
Member

njsmith commented Dec 30, 2015

np.random.randint(5, [10, None])

I wouldn't bother... just tell people to write randint([5, 0], [10, 5]) if that's what they want. (It'll be infinitely less confusing.)

@gfyoung
Copy link
Contributor Author

gfyoung commented Dec 30, 2015

Okay, that will make things a lot easier. Will add that to documentation.

@gfyoung
Copy link
Contributor Author

gfyoung commented Dec 30, 2015

Documentation added, and Travis is happy. I have no idea how this will gel with #6824 or #6869, but as it stands, unless there are some major style/functional issues I'm missing, I think it should be good to go.

@gfyoung
Copy link
Contributor Author

gfyoung commented Jan 3, 2016

In light of the email discussion, would it be safe to close this PR along with the related issue (#6745)?

@homu
Copy link
Contributor

homu commented Jan 3, 2016

☔ The latest upstream changes (presumably #6910) made this pull request unmergeable. Please resolve the merge conflicts.

@charris
Copy link
Member

charris commented Jan 3, 2016

@gfyoung I'll close this as it would need a complete rewrite at this point. I'll leave the issue open just in case someone wants to pursue it. Thanks for your work here.

@charris charris closed this Jan 3, 2016
@gfyoung gfyoung deleted the randint_arg_broadcast branch January 3, 2016 23:23
@gfyoung gfyoung restored the randint_arg_broadcast branch January 4, 2016 00:25
bashtage added a commit to bashtage/numpy that referenced this pull request Aug 31, 2017
Remove old rand int helpers
Add tests for array versions
Remove mask generator

xref numpy#6938
xref numpy#6902
xref numpy#6745
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants