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

change argument order for rand! (fix #8246) #9092

Merged
merged 1 commit into from
Nov 21, 2014
Merged

Conversation

rfourquet
Copy link
Member

The current API is rand!([rng], [r::Range], A). There seems to be an almost consensus (cf. #8246) to put r::Range as last argument, and a preference to keep rng before A, i.e: rand!([rng], A, [::Range]). This PR implements this solution, but can easily be changed to rand!(A, [rng], [::Range]) if a decision is made this way.
Note: r::Range is also changed to r::AbstractArray, to match rand.

The API to fill randomly an array A is changed
from rand!([rng], [::Range], A) to rand!([rng], A, [::AbstractArray]).
Enabling [::AbstractArray] instead of only [::Range] depended on choosing first
the argument order.
@rfourquet
Copy link
Member Author

Note that this should not break code (only deprecation warnings in e.g. rand!(1:9, A)).

@johnmyleswhite
Copy link
Member

I'll switch my vote to get things moving faster.

ViralBShah added a commit that referenced this pull request Nov 21, 2014
@ViralBShah ViralBShah merged commit ca7f6d1 into master Nov 21, 2014
@ViralBShah
Copy link
Member

@rfourquet Thanks for all the rapid contributions! You've really helped move things along on our RNG front.

@ViralBShah ViralBShah added the randomness Random number generation and the Random stdlib label Nov 22, 2014
@rfourquet rfourquet deleted the rf/randbang-arg-order branch November 22, 2014 07:23
@rfourquet
Copy link
Member Author

:-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
randomness Random number generation and the Random stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants