Conversation
| } | ||
|
|
||
| #[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
tests/snippets/stdlib_random.py
Outdated
| 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.
| #[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.
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.
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!
| 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).
|
@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? |
|
Sure |
4 participants
Functions for integers, functions for sequences works. But, Real-valued distributions is still working on progress.