Repository navigation
Implement "random" module (WIP) - #1571
malkoG wants to merge 2 commits into
Conversation
Sorry, something went wrong.
| } | ||
|
|
||
| #[pymethod] | ||
| fn seed(&self, n: Option<usize>, vm: &VirtualMachine) -> PyResult { |
There was a problem hiding this comment.
You can make n an Option<u64> to avoid the cast
Sorry, something went wrong.
| right = [2, 7, 3, 5, 8, 4, 6, 9, 0, 1] | ||
| random.shuffle(left) | ||
|
|
||
| assert all([l == r for l, r in zip(left, right) ]) |
There was a problem hiding this comment.
Isn't this same as assert left == right? Same for other tests too.
Sorry, something went wrong.
| #[pyclass(name = "Random")] | ||
| #[derive(Debug)] | ||
| struct PyRandom { | ||
| rng: RefCell<SmallRng> |
There was a problem hiding this comment.
I would strongly recommend to use StdRng as a default instead, it is a much safer choice, because it is not predictable.
Sorry, something went wrong.
There was a problem hiding this comment.
It would probably be possible to have an enum inside the RefCell with both StdRng and SmallRng as variants, and only switch to SmallRng when seed is called.
Sorry, something went wrong.
There was a problem hiding this comment.
SmallRng is not the Mersenne Twister, so it does not make sense to switch to it for compatibility with CPython. Other than that, I agree!
Sorry, something went wrong.
| None => SmallRng::from_entropy(), | ||
| Some(n) => { | ||
| let seed = n as u64; | ||
| SmallRng::seed_from_u64(seed) |
There was a problem hiding this comment.
It would be nice to support larger seeds (via bigints), but this requires more work and could be done later (likely in a backwards incompatible way).
Sorry, something went wrong.
|
@malkoG I'd be willing to take over this PR, could I have your permission to use the code you've written as a base? |
Sorry, something went wrong.
|
Sure |
Sorry, something went wrong.
Functions for integers, functions for sequences works. But, Real-valued distributions is still working on progress.