Skip to content

Add IndexedRandom::choose_multiple_array, index::sample_array #1453

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 7 commits into from
Jun 4, 2024

Conversation

dhardy
Copy link
Member

@dhardy dhardy commented May 21, 2024

  • Added a CHANGELOG.md entry

Summary

#982 requests the addition of choose_multiple_fill, but for the given use-case (fixed-length output), an array appears more appropriate.

Details

This requires additional code in rand::seq::index. Under the assumption that the array length is small, it is reasonable to skip the algorithm-selection-logic and always use Floyd's alg for the new fn.

Additional

Two new modules under rand::seq for cleaner code organisation.

@dhardy dhardy requested a review from newpavlov May 30, 2024 08:45
Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

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

Maybe it's also worth to add (in a separate PR) borrowing choose methods?

@dhardy dhardy merged commit ca9e119 into rust-random:master Jun 4, 2024
14 checks passed
@dhardy
Copy link
Member Author

dhardy commented Jun 4, 2024

@newpavlov what do you mean? IndexedRandom::choose already borrows &self.

Do you mean choose_and_remove? If so, it will have more restrictions on the containing type than Index<usize> (or mut variant); e.g. it could be implemented on a Vec but would be inefficient.

@newpavlov
Copy link
Member

Never mind, I misunderstood the API.

@dhardy dhardy deleted the work branch July 8, 2024 09:55
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants