Skip to content

Rename fns choose_multiple* → sample*; add choose*_iter #1632

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

dhardy
Copy link
Member

@dhardy dhardy commented Apr 19, 2025

  • Added a CHANGELOG.md entry

Fixes #1620 by:

  • Renaming IndexedRandom::choose_multiple*sample*
  • Renaming IteratorRandom::choose_multiple*sample*
  • Adding IndexedRandom::choose_iter, choose_weighted_iter

I think I'm happy to go with this. @vks, @benjamin-lieser review please.

@dhardy dhardy force-pushed the push-kvlpklxwrypt branch from e67fa45 to 534b292 Compare April 19, 2025 12:14
@dhardy
Copy link
Member Author

dhardy commented Apr 19, 2025

Note: I don't think we should add choose_mut_iter. Even if we wanted to, we couldn't do so using the Iterator trait; we'd need a streaming/borrowing iterator trait. The choose_iter fn here returns the same lifetime (that of the iterator) for each next sample.

I briefly considered deprecating all the choose_mut fns but a GitHub search shows some uses.

@dhardy dhardy requested review from vks and benjamin-lieser April 25, 2025 07:51
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.

choose_multiple should return Result or Option
2 participants