-
Notifications
You must be signed in to change notification settings - Fork 63
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
Remove rand
hacks and use make
instead
#1388
Comments
Will this fix the problem observed in JuliaRandom/RandomExtensions.jl#13? |
Yes, exactly that is what I meant. Thanks for pointing to the issue. |
I am also not sure I understand the ramifications. Will we have to use |
As far as I have understood it (and my testing showed nothing contradicting that), this is the only visible change to the user. |
I think writing |
I too think that rand(make(..)) cannot fly. We've only recently looked at the creation of random mpolys and I am afraid a normal user will not be able to create them using the current design> The proposed it worse. It's cool for experts but impossible for users |
I also don't like See oscar-system/Oscar.jl#2896 for a different proposal. |
rand
hacks to not need make
rand
hacks and use make
instead
Removing these Hacks at a later stage would be a breaking change, so I think we need to address this part before 1.0 (even without a final concept and implementation of oscar-system/Oscar.jl#2896). |
All over AA, there are custom
rand
methods, that insert the RNG as a first argument, e.g.AbstractAlgebra.jl/docs/src/rand.md
Lines 117 to 125 in 06b8630
In particular, they change
rand(make(S, 1:3, -10:10))
torand(S, 1:3, -10:10)
.The
RandomExtensions
documentation strongly suggests to always usemake
in calls and not use the hack, seehttps://github.com/JuliaRandom/RandomExtensions.jl/blob/f5d9723da41cb85f8c8be6ae21cb220ea3101ccc/README.md?plain=1#L35-L42
Furthermore, according to
Aqua.test_ambiguity(AbstractAlgebra)
, these customrand
methods are part of 587 of the 622 ambiguities total!Most of these methods have been there for at least 3 years untouched, some even longer (just been moved around between files a few times). And I think due to the above reasons, it is time for some refactoring.
The text was updated successfully, but these errors were encountered: