-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
add rand(::Pair) #28704
add rand(::Pair) #28704
Conversation
I'm not sure I entirely like this. I think of pairs as having more semantics than just a two element collection of its elements. Frankly, I'm not sure what else it would mean in this context, but it seems like we may want to have it available for the future. |
Thanks for raising this point. I for one really wanted to use this syntax in #24912, for combining distribution. For example, |
I say wait and see. If you want this functionality you can always splat the pair into a tuple. |
Turning the argument upside down, shouldn't a 2-Tuple be used in #24912 (which can be extended to tuples at some point), as there is no more "semantics than just a two element collection of its elements" to the use? |
Ah, yes, perfect. |
FWIW, I implemented the proposition from my comment above in RandomExtension.jl, e.g.
As said above, it's not well inferred, but it's not that bad thanks to inlining. It's just a surface API on top of a tightly inferred one. I'm in favor of still not merging this PR, and wait and see. |
I don't see the advantage of this semantics over just constructing an element from randomly generated values (the alternative proposed above) or picking an element from any collection (but a I like to think of |
I'm not sure I understand this... if you mean that e.g.
The |
Just to clarify:
Also, from a broader perspective, we are essentially talking about elements of a DSL to describe distributions over Julia types. This is a pretty complex task and IMO should be first hammered out in a package, to see how people use it and clear up the design issues. So while I don't see the use case for the |
Ok, thanks for the clarifications!
Type piracy is not a problem here, as this is intended as an experimental feature, which may be included in |
What is the idea that |
It's for composability (and sometimes efficiency). This is the equivalent of calling the When we want to generate a random "scalar" of type For generating a string we have e.g.
would be equivalent to
(but more efficient in this case as it uses the Similarly, a
vs
Note that this generic |
Can't argue with that! I was confused because this is your PR. Let's just not define rand(::Pair) 😃 |
(Just to be sure this was clear, this PR and my previous post define two competing meaning for |
I think that's a reasonable goal, but I find the that just creating a |
This is similar to #25278, which adds
rand(::Tuple)
.Pair
has the attributes of a collection, so there is no reason to exclude it fromrand
able objects.