Skip to content

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

Merged
dhardy merged 6 commits intomasterfrom
push-kvlpklxwrypt
Sep 4, 2025
Merged

Rename fns choose_multiple* → sample*; add choose*_iter#1632
dhardy merged 6 commits intomasterfrom
push-kvlpklxwrypt

Conversation

@dhardy
Copy link
Member

@dhardy dhardy commented Apr 19, 2025 •
edited
Loading

  • 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 benjamin-lieser and vks April 25, 2025 07:51
@dhardy dhardy added the B-API Breakage: API label Jun 1, 2025
dhardy added a commit that referenced this pull request Aug 12, 2025
@dhardy
Merge master branch into #1632
11df11d
dhardy
dhardy commented Aug 12, 2025
View reviewed changes
@dhardy dhardy mentioned this pull request Aug 12, 2025
Merged
1 task
@dhardy
Copy link
Member Author

dhardy commented Sep 4, 2025

Rebased. Ready for merge (hopefully).

@dhardy dhardy merged commit 34b2e45 into master Sep 4, 2025
16 checks passed
@dhardy dhardy deleted the push-kvlpklxwrypt branch September 4, 2025 09:10
@Dieshen Dieshen mentioned this pull request Mar 5, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vks vks vks approved these changes

@benjamin-lieser benjamin-lieser Awaiting requested review from benjamin-lieser

Assignees

No one assigned

Labels

B-API Breakage: API

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

choose_multiple should return Result or Option

2 participants

@dhardy @vks