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

two small changes from #24912 #25319

Merged
merged 2 commits into from
Jan 4, 2018
Merged

two small changes from #24912 #25319

merged 2 commits into from
Jan 4, 2018

Conversation

rfourquet
Copy link
Member

  • rename CloseOpen -> CloseOpen01, Close1Open2 -> CloseOpen12

This allows for more consistency, and eventually to add a CloseOpen abstract type which allows to generate random values in a more general interval (rather than just [0,1) or [1,2).

  • make Sampler{E} encode the type E of elements which are generated

This allows to get concretely typed arrays in some cases (cf. commit message).

Before, a call like `rand(rng, Sampler(rng, 1:10), 3)`
generated an `Array{Any,1}`, so a way to get the `eltype`
of a Sampler is necessary. Instead of changing Sampler -> Sampler{E},
implementing appropriate eltype methods would have been possible,
to keep the helper Sampler subtypes more flexible, but it seemed
to be simpler this way.
@rfourquet rfourquet added the randomness Random number generation and the Random stdlib label Dec 29, 2017
@JeffBezanson
Copy link
Sponsor Member

Seems harmless enough to me. Should this be merged?

@rfourquet
Copy link
Member Author

It should indeed be harmless, improve the type of some randomly generated arrays, and be more future-proof: even if those types are not yet exported from Random, they will probably be used; in particular I think parameterizing Sampler{T} with the type of generated values (T) is an improvement in clarity.

@JeffBezanson JeffBezanson merged commit e3b7b18 into master Jan 4, 2018
@JeffBezanson JeffBezanson deleted the rf/rand/unleash-remains branch January 4, 2018 16:08
@rfourquet
Copy link
Member Author

Thanks!

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.

2 participants