Skip to content

add shuffle(::NTuple) to Random #56906

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 29 commits into from
Aug 16, 2025
Merged

add shuffle(::NTuple) to Random #56906

merged 29 commits into from
Aug 16, 2025

Conversation

nsajko
Copy link
Member

@nsajko nsajko commented Dec 26, 2024

Fixes #56728

@nsajko nsajko added randomness Random number generation and the Random stdlib stdlib Julia's standard library feature Indicates new feature / enhancement requests labels Dec 26, 2024
nsajko
nsajko commented Dec 26, 2024
View reviewed changes
@rfourquet rfourquet added needs docs Documentation for this change is required needs news A NEWS entry is required for this change labels May 9, 2025
@nsajko nsajko force-pushed the shuffle_ntuple branch from 213f598 to 7bb811d Compare May 9, 2025 10:29
nsajko
nsajko commented May 9, 2025
View reviewed changes
nsajko
nsajko commented May 9, 2025
View reviewed changes
@nsajko nsajko force-pushed the shuffle_ntuple branch from 70fa8ac to 4577162 Compare May 9, 2025 10:39
@nsajko nsajko added status: waiting for PR reviewer DO NOT MERGE Do not merge this PR! and removed needs news A NEWS entry is required for this change needs docs Documentation for this change is required status: waiting for PR reviewer labels May 9, 2025
@nsajko
Copy link
Member Author

nsajko commented May 9, 2025 •
edited
Loading

Unlike shuffle!, randperm! is currently restricted to Array, not admitting Memory, so this PR now depends on randperm! supporting Memory. Clearly it would be good to support more types than just Array with randperm! anyway. Will probably make another PR for that soon.

EDIT: decided to just forgo randperm! for now

nsajko
nsajko commented May 9, 2025
View reviewed changes
@nsajko nsajko removed the DO NOT MERGE Do not merge this PR! label May 9, 2025
rfourquet
rfourquet reviewed May 9, 2025
View reviewed changes
nsajko
nsajko commented May 9, 2025
View reviewed changes
nsajko
nsajko commented May 9, 2025
View reviewed changes
rfourquet
rfourquet reviewed May 12, 2025
View reviewed changes
@rfourquet rfourquet added the needs compat annotation Add !!! compat "Julia x.y" to the docstring label May 12, 2025
@rfourquet
Copy link
Member

I'm surprised to see that your implementation allocates on the master branch, but not on v1.12, do you have an idea why that is the case? Not necessariy a problem though, it seems to be roughly the same speed.

@nsajko
Copy link
Member Author

nsajko commented Jun 10, 2025

The PR #58596:

  • made randperm! support Memory, so it's now used instead of shuffle!

  • caused a merge conflict in NEWS.md, now fixed

@nsajko nsajko mentioned this pull request Jun 13, 2025
Closed
@nsajko
Copy link
Member Author

nsajko commented Jun 15, 2025

bump?

adienes
adienes reviewed Jun 16, 2025
View reviewed changes
@nsajko
Copy link
Member Author

nsajko commented Jun 25, 2025

ping

@nsajko
Copy link
Member Author

nsajko commented Jul 27, 2025

bump

@nsajko
Copy link
Member Author

nsajko commented Aug 9, 2025

ping

adienes
adienes reviewed Aug 11, 2025
View reviewed changes
adienes
adienes reviewed Aug 11, 2025
View reviewed changes
@nsajko @adienes
elide the typeassert
4b3c141 
Co-authored-by: Andy Dienes <[email protected]>
@adienes adienes added merge me PR is reviewed. Merge when all tests are passing and removed status: waiting for PR reviewer labels Aug 15, 2025
@IanButterworth IanButterworth merged commit f4af91d into JuliaLang:master Aug 16, 2025
7 checks passed
@nsajko nsajko deleted the shuffle_ntuple branch August 16, 2025 21:24
@nsajko nsajko removed the merge me PR is reviewed. Merge when all tests are passing label Aug 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfourquet rfourquet rfourquet left review comments

@adienes adienes adienes approved these changes

+1 more reviewer

@barucden barucden barucden left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
feature Indicates new feature / enhancement requests randomness Random number generation and the Random stdlib stdlib Julia's standard library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support shuffle(::NTuple)
5 participants
@nsajko @rfourquet @barucden @adienes @IanButterworth