-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[stdlib] Introduce random.shuffle
for List
#3327
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
Conversation
For concerns regarding the Fish 8000 er-Yates algorithm, one can refer to a corresponding issue in the CPython repository. In summary, the Python community appears to be satisfied with this implementation. |
7789dee
to
87a6757
Compare
Can we set a seed to make the tests deterministic? |
Are you suggesting the same seed for all tests? Is the concern about potential failing assertions due to randomness? It might be beneficial to introduce some seeded tests, or perhaps modify some existing ones to seeded. Currently we rely on an extremely low chance of a shuffled list resulting in the same order to assert success. There is more confidence that a failed test case indicates an ineffective shuffle. Alternatively, using property tests, followed by sorting and checking for equality with the original list, might be more reliable. What do you think? |
I don't have a strong opinion about this, and if we do move to explicit seeding, we don't have to do it in this PR. I'm just taking the chance to bring it up. My main point is: if the tests are deterministic, we can simplify them into something like this: seed()
var l = List(1, 2, 3, 4)
shuffle(l)
assert_equal(l, List(2, 3, 1, 4)) # we know beforehand the result we will get |
Signed-off-by: Joshua James Venter <venter.joshua@gmail.com>
Signed-off-by: Joshua James Venter <venter.joshua@gmail.com>
3389dce
to
b338653
Compare
Signed-off-by: Joe Loser <joe@modular.com>
!sync |
✅🟣 This contribution has been merged 🟣✅ Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours. We use Copybara to merge external contributions, click here to learn more. |
[External] [stdlib] Introduce `random.shuffle` for `List` Introduce `random.shuffle` for `List`. Implementation follows the Fisher-Yates shuffle. Co-authored-by: Joshua James Venter <venter.joshua@gmail.com> Closes #3327 MODULAR_ORIG_COMMIT_REV_ID: 911eadee8d34911f653e8c69f85fcd89f5cae344
Landed in ba0f514! Thank you for your contribution 🎉 |
[External] [stdlib] Introduce `random.shuffle` for `List` Introduce `random.shuffle` for `List`. Implementation follows the Fisher-Yates shuffle. Co-authored-by: Joshua James Venter <venter.joshua@gmail.com> Closes #3327 MODULAR_ORIG_COMMIT_REV_ID: 911eadee8d34911f653e8c69f85fcd89f5cae344
Introduce
random.shuffle
forList
. Implementation follows the Fisher-Yates shuffle.