Rename srand to Random.seed! #28295

MoonCoral · 2018-07-26T20:23:42Z

Note that there are a bunch of deprecation warnings concerning Base.Random when running tests.
Also this causes Random.seed(seed) to be a common occurrence which could lead to confusion.
I didn't touch the occurrences in Base as they seem to be actually calling the c srand.

(This is my first PR)

ararslan · 2018-07-26T21:02:07Z

stdlib/Random/src/deprecated.jl

@@ -37,3 +37,6 @@ end

 # PR #25668
 @deprecate RandomDevice(unlimited::Bool) RandomDevice(; unlimited=unlimited)
+
+# PR #Current
+@deprecate srand Random.seed


I believe this should have a false at the end to avoid exporting it, i.e.

@deprecate srand Random.seed false

I also don't think this form is supported, I think you need to write a custom message with depwarn(...)

No this works okay:

julia> module A f() = 1 Base.@deprecate g A.f false end Main.A julia> using .A julia> A.g() ┌ Warning: `g` is deprecated, use `A.f` instead. │ caller = top-level scope at none:0 └ @ Core none:0 1

Right, it is the other way around that does not work

I will update it when I wake up tomorrow. In the meantime, does anyone know where the:

WARNING: importing deprecated binding Base.Random into Test. Base.Random is deprecated, run `using Random` instead ERROR: LoadError: error compiling top-level scope: deprecated binding: Base.Random WARNING: importing deprecated binding Base.Random into Test. Base.Random is deprecated, run `using Random` instead ERROR: LoadError: error compiling top-level scope: deprecated binding: Base.Random

and similar come from? As they seem to be the main culprit to the tests failing.

In Test.jl you need using Random instead of just using Random: ..., which only imports specific items from Random (and not Random itself).

Got it. Thank you.

you need using Random instead of just using Random: ...,

In this case, import Random would be slightly better I think (imports only the binding Random itself), as there is already a line using Random: ... with explicit functions. But not worth changing again.
BTW, wouldn't it make sense that using Random: ... also imports the Random binding itself?

Sacha0 · 2018-07-26T21:21:05Z

Welcome @MoonCoral! Wonderful first contribution! 🎉

MoonCoral · 2018-07-27T11:46:35Z

This should be it. Tell me if I should squash the commits.

rfourquet · 2018-07-27T18:47:17Z

NEWS.md

@@ -1321,6 +1321,8 @@ Deprecated or removed

  * `ndigits(n, b, [pad])` is deprecated in favor of `ndigits(n, base=b, pad=pad)` ([#27908]).

+  * `srand` is deprecated in favor of the unexported `Random.seed` ([#27726]).


I believe the PR number should be mentioned here

The issue number is sufficient.

fredrikekre · 2018-07-27T19:00:44Z

stdlib/Random/src/deprecated.jl

@@ -37,3 +37,6 @@ end

 # PR #25668
 @deprecate RandomDevice(unlimited::Bool) RandomDevice(; unlimited=unlimited)
+
+# PR #28295
+@deprecate srand Random.seed false


Add a deprecation for guardsrand -> guardseed as well? It was introduced in the 0.7 cycle but I know some packages use it so could be good to add.

guardsrand was unexported in #27942, without deprecation...

ararslan

Don't worry about squashing the commits, we can do that on merge. I have one outstanding comment, and other than that this looks great to me. Thanks for taking it on!

ararslan · 2018-07-27T19:02:07Z

stdlib/Random/src/deprecated.jl

@@ -37,3 +37,6 @@ end

 # PR #25668
 @deprecate RandomDevice(unlimited::Bool) RandomDevice(; unlimited=unlimited)
+
+# PR #28295
+@deprecate srand Random.seed false


I was actually wrong about false; that avoids exporting srand, not Random.seed. What you had originally was correct. Sorry about that!

rfourquet · 2018-07-27T19:04:13Z

I would like to use this renaming opportunity to suggest Random.seed! instead. This was not mentioned in the linked issue, but was very briefly discussed at #9105 (comment).
It's true that we don't use the exclamation mark in rand even though the RNG state is modified. But I woud argue that this is the expected way of modifying the state, and the focus is on the produced random values (and we prefer reserving ! for our current rand!). In srand, the focus is on the RNG itself, with the goal to mutate it, so I think it deserves a !. For example, in the "Random.jl" __init__ function, we now have this call: seed(), on a line by itself; we usually don't see in Julia such a function without ! called only for its side effect; it really looks like this call does nothing.
Morever, in seed!(rng, seed), the ! helps a bit understand seed! as a verb versus seed as a noun.

JeffBezanson · 2018-07-27T19:10:01Z

That's a good point; particularly if this is generalized to operate on an RNG, seed!(rng, val) makes more sense.

Deprecation has been fixed

MoonCoral · 2018-07-27T20:37:52Z

That went badly :(

MoonCoral · 2018-07-27T20:55:31Z

My apologies for my clumsy way to fix the merge conflict. I think all changes on my side should be fine now, so I'll refrain from touching git further and making things worse.

rfourquet · 2018-07-28T10:22:32Z

I think all changes on my side should be fine now, so I'll refrain from touching git further and making things worse.

In the diff here, many unrelated changes still appear, so I'm afraid you may have to touch git a bit more!

MoonCoral · 2018-07-28T11:55:23Z

I believe I managed to clean this up.

rfourquet

LGTM, thanks!

ararslan · 2018-07-28T23:22:15Z

stdlib/Test/src/Test.jl

 state of the global RNG as it was before."
-function guardsrand(f::Function, r::AbstractRNG=GLOBAL_RNG)
+function guardseed(f::Function, r::AbstractRNG=GLOBAL_RNG)


Oh I see that was already discussed. Never mind.

ararslan reviewed Jul 26, 2018

View reviewed changes

ararslan added domain:randomness Random number generation and the Random stdlib kind:deprecation This change introduces or involves a deprecation needs news A NEWS entry is required for this change labels Jul 26, 2018

mbauman removed the needs news A NEWS entry is required for this change label Jul 27, 2018

rfourquet reviewed Jul 27, 2018

View reviewed changes

fredrikekre reviewed Jul 27, 2018

View reviewed changes

ararslan previously requested changes Jul 27, 2018

View reviewed changes

MoonCoral changed the title ~~Rename srand to Random.seed~~ Rename srand to Random.seed! Jul 27, 2018

Olivier added 13 commits July 28, 2018 12:38

Changed implementation name of srand -> seed and removed export.

a707aac

Switched usage in stdlb/Random and added deprecation.

a0f00aa

Change usage in stdlib.

4a1291f

Changed usage in test.

86a0563

Changed usage in doc.

b9a3b01

Add explicit 'Random'.seed().

28a76c4

Amend Deprecation.

91f5f3c

Ensure proper 'using Random's.

049bc31

Add News.

31cf36b

Fix typo from earlier commit.

5b64541

Change internal Test.guardsrand to Test.guardseed to match Random.seed.

f242e57

Change a using statment where I tried to be to smart.

4098622

Fix error in my News commit.

4fd00b5

Olivier Thill added 3 commits July 28, 2018 12:42

Change deprecation again.

75ff13e

Renamed Random.seed to Random.seed!

7b42b44

Fix Merge Issue.

d3142e0

rfourquet approved these changes Jul 28, 2018

View reviewed changes

ararslan reviewed Jul 28, 2018

View reviewed changes

Merge branch 'master' into srand_Random_seed

c632980

ararslan merged commit cf78a31 into JuliaLang:master Jul 29, 2018

fredrikekre mentioned this pull request Jul 29, 2018

srand -> Random.seed! JuliaComputing/Deprecations.jl#81

Closed

rdeits mentioned this pull request Aug 4, 2018

srand is now Random.seed! JuliaLang/Compat.jl#601

Merged

KristofferC pushed a commit that referenced this pull request Feb 11, 2019

Rename srand to Random.seed! (#28295)

821f91b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename srand to Random.seed! #28295

Rename srand to Random.seed! #28295

MoonCoral commented Jul 26, 2018 •

edited

Loading

ararslan Jul 26, 2018

fredrikekre Jul 26, 2018

ararslan Jul 26, 2018

fredrikekre Jul 26, 2018

MoonCoral Jul 26, 2018 •

edited

Loading

JeffBezanson Jul 26, 2018

MoonCoral Jul 26, 2018

rfourquet Jul 28, 2018

Sacha0 commented Jul 26, 2018

MoonCoral commented Jul 27, 2018 •

edited

Loading

rfourquet Jul 27, 2018

JeffBezanson Jul 27, 2018

fredrikekre Jul 27, 2018

rfourquet Jul 27, 2018

ararslan left a comment

ararslan Jul 27, 2018

rfourquet commented Jul 27, 2018

JeffBezanson commented Jul 27, 2018

MoonCoral commented Jul 27, 2018

MoonCoral commented Jul 27, 2018

rfourquet commented Jul 28, 2018

MoonCoral commented Jul 28, 2018

rfourquet left a comment

This comment was marked as outdated.

ararslan Jul 28, 2018

		@@ -1321,6 +1321,8 @@ Deprecated or removed

		* `ndigits(n, b, [pad])` is deprecated in favor of `ndigits(n, base=b, pad=pad)` ([#27908]).

		* `srand` is deprecated in favor of the unexported `Random.seed` ([#27726]).

Rename srand to Random.seed! #28295

Rename srand to Random.seed! #28295

Conversation

MoonCoral commented Jul 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MoonCoral Jul 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sacha0 commented Jul 26, 2018

MoonCoral commented Jul 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ararslan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfourquet commented Jul 27, 2018

JeffBezanson commented Jul 27, 2018

MoonCoral commented Jul 27, 2018

MoonCoral commented Jul 27, 2018

rfourquet commented Jul 28, 2018

MoonCoral commented Jul 28, 2018

rfourquet left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

Choose a reason for hiding this comment

MoonCoral commented Jul 26, 2018 •

edited

Loading

MoonCoral Jul 26, 2018 •

edited

Loading

MoonCoral commented Jul 27, 2018 •

edited

Loading