-
-
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
Replace the use of randbool() #9569
Conversation
0796c87
to
b52f58e
Compare
Cc: @carlobaldassi As discussed in #9105, I have only addressed |
@@ -259,3 +259,5 @@ const base64 = base64encode | |||
|
|||
@deprecate sizehint(A, n) sizehint!(A, n) | |||
|
|||
@deprecate randbool() rand(Bool) | |||
@deprecate randbool(x...) bitrand(x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are these different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because randbool()
produces a Bool
, whereas bitrand()
produces a BitArray
(as it should).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the forwarding of randbool
taking an AbstractRNG
won't work as is, nor randbool((2,3))
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you're right, did not realize randbool()
with no inputs gave a Bool
, nevermind. Glad we're deprecating that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rfourquet Thanks. I have added all the deprecation cases, and they work correctly now.
b52f58e
to
66ec067
Compare
@deprecate randbool(dims) bitrand(dims) | ||
@deprecate randbool(dims...) bitrand(dims) | ||
@deprecate randbool(r, dims) bitrand(r, dims) | ||
@deprecate randbool(r, dims...) bitrand(r, dims) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
randbool(1, 2, 3)
won't work, and randbool(MersenneTwister())
won't work as expected, I think you can't avoid using type annotations here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I keep losing track of all the different method signatures!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we not have rand(r::AbstractRNG, t::Type)
so that different RNGs can be used to generate random values of various integer/boolean types? It appears not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind. We do have it - but I was trying in 0.3.4, which was open in another window.
The linux failure is a segfault, and appears unrelated (but worrisome). |
I used almost the exact same wording in another PR, though when it happened on OSX. Probably related to #9544 #9570, the |
66ec067
to
e8eb7e1
Compare
Now it happened in OS X here... |
For a random Bool, instead of randbool(), use rand(Bool) Instead of randbool([rng], dims), use bitrand([rng], dims) Add randbool deprecation to NEWS
e8eb7e1
to
ff69e6f
Compare
Replace the use of randbool()
As per JuliaLang/julia#9569, `randbool()` has been deprecated in favor of `rand(Bool)`.
As per JuliaLang/julia#9569, `randbool()` has been deprecated in favor of `rand(Bool)`.
Deprecate randbool (#9105).
For a random Bool, instead of randbool(), use rand(Bool)
Instead of randbool(dims), use bitrand(dims)