Skip to content
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

Add Iterator::choose_stable() #1057

Merged
merged 2 commits into from Oct 17, 2020
Merged

Conversation

kevincox
Copy link
Contributor

This function is similar to Iterator::choose() except that given a PRNG and any iterator of the same length it will always select the same element and make the same calls to the PRNG.

Closes #1051

@vks
Copy link
Collaborator

vks commented Oct 10, 2020

Thanks for taking care of this! I did not yet look at the code, but the documentation looks good. Maybe we should mention now in the docs of choose that choose_stable exists?

@kevincox
Copy link
Contributor Author

I added a hint about choose_stable to the choose docs.

Copy link
Collaborator

@kazcw kazcw left a comment

Choose a reason for hiding this comment

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

Generally looks good. I suggested some tweaks relating to 32/64-bit consistency and testing value stability.

src/seq/mod.rs Outdated Show resolved Hide resolved
src/seq/mod.rs Outdated Show resolved Hide resolved
src/seq/mod.rs Show resolved Hide resolved
Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

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

LGTM other than a simple typo.

Could you rebase? #1056 should have fixed that test failure.

src/seq/mod.rs Outdated Show resolved Hide resolved
This function is similar to Iterator::choose() except that given a PRNG and any iterator of the same length it will always select the same element and make the same calls to the PRNG.

Closes rust-random#1051
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.

Stable version of rand::seq::IteratorRandom::choose
4 participants