Skip to content

drop base rand shadowing#56

Closed
isaacsas wants to merge 1 commit intomasterfrom
drop_base_rand_shadowing
Closed

drop base rand shadowing#56
isaacsas wants to merge 1 commit intomasterfrom
drop_base_rand_shadowing

Conversation

@isaacsas
Copy link
Member

Let's see if tests pass, but I don't see why these are even necessary.

@isaacsas isaacsas changed the title drop base shadowing drop base rand shadowing Sep 10, 2025
Comment on lines 12 to 13
randexp(rng::PassthroughRNG) = Random.randexp()
randn(rng::PassthroughRNG) = Random.randn()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since they are not imported, I assume one needs

Suggested change
randexp(rng::PassthroughRNG) = Random.randexp()
randn(rng::PassthroughRNG) = Random.randn()
Random.randexp(rng::PassthroughRNG) = Random.randexp()
Random.randn(rng::PassthroughRNG) = Random.randn()

(and the line above which I can't comment on) since otherwise calls of randexp etc. below for other RNGs than PassthroughRNG will fail.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe these should just be internal wrappers with a slightly different name? Like _rand, _randexp, etc? That would make clear these aren't meant to be public dispatches of Random.rand and such?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the simplest would be to just define Random.rand, Random.randexp, and Random.randn for PassthroughRNG. For all other RNGs, the code will work automatically, without having to go through some internal function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants