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.

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

Make randperm and randcycle use the argument's type. #29670

Merged
merged 1 commit into from Nov 29, 2018
Merged

Make randperm and randcycle use the argument's type. #29670

merged 1 commit into from Nov 29, 2018

Conversation

tpapp
Copy link
Contributor

@tpapp tpapp commented Oct 16, 2018

Make randperm and randcycle use the argument's type, so that

eltype(randperm(T(n))) === T

This is potentially very useful when generating large random permutations, as using Int32 instead of Int64 can halve memory usage.

Removes tests for eltype being Int. This is potentially controversial as it may break things (so discussion is needed), but I did not see it documented anywhere as part of the API so I don't consider this breaking.

@tpapp
Make randperm and randcycle use the argument's type.
398aa08 
Remove tests for eltype being `Int`. This is potentially controversial
as it may break things, but I did not see it documented anywhere as
part of the API.
@fredrikekre fredrikekre added the domain:randomness Random number generation and the Random stdlib label Oct 16, 2018
@StefanKarpinski
Copy link
Sponsor Member

Seems like a strict improvement to me; could technically break someone's code so "minor change".

@StefanKarpinski StefanKarpinski added the kind:minor change Marginal behavior change acceptable for a minor release label Oct 16, 2018
@JeffBezanson JeffBezanson added the status:needs news A NEWS entry is required for this change label Nov 29, 2018
@JeffBezanson JeffBezanson merged commit caea73d into JuliaLang:master Nov 29, 2018
@JeffBezanson JeffBezanson removed the status:needs news A NEWS entry is required for this change label Nov 29, 2018
@tpapp tpapp deleted the tp/randperm-narrow branch November 29, 2018 19:54
@rfourquet
Copy link
Member

For reference, this was the original behavior (implemented by Stefan IIRC), which I changed in #22723, without much discussion (no less than here though), but here is my quote from there:

The integer type determing the array type makes sense in a way, when considering this Int is the upper bound of the returned array; but this is a bit too implicit for me. On the other hand, this integer can be seen simply as the length of the array, like many other functions in base, for which non-Int integers are accepted only for convenience, but without affecting the type of the result.

fredrikekre added a commit that referenced this pull request Nov 30, 2018
@fredrikekre
Compat annotations for #29670 (randperm and randcycle return type).
d4159ed
fredrikekre added a commit that referenced this pull request Dec 1, 2018
@fredrikekre
Compat annotations for #29670 (randperm and randcycle return type).
e6474b9
fredrikekre added a commit that referenced this pull request Dec 1, 2018
@fredrikekre
Compat annotations for #29670 (randperm and randcycle return type).
9188bf6
fredrikekre added a commit that referenced this pull request Dec 3, 2018
@fredrikekre
Compat annotations for #29670 (randperm and randcycle return type).
1723f2c
fredrikekre added a commit that referenced this pull request Dec 4, 2018
@fredrikekre
Compat annotations for #29670 (randperm and randcycle return type).
1caba8a
fredrikekre added a commit that referenced this pull request Dec 4, 2018
@fredrikekre
Compat annotations for #29670 (randperm and randcycle return type).
4b4747d
fredrikekre added a commit that referenced this pull request Dec 5, 2018
@mortenpi @fredrikekre
Addition of NEWS and compat admonitions for important
cceb675 
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
fredrikekre added a commit that referenced this pull request Dec 5, 2018
@mortenpi @fredrikekre
Addition of NEWS and compat admonitions for important
ebbabf5 
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
fredrikekre added a commit that referenced this pull request Dec 5, 2018
@fredrikekre @mortenpi
Compat admonitions and NEWS for Julia 1.1 (#30230)
6175bd9 
Addition of NEWS and compat admonitions for important changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.


Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfourquet rfourquet Awaiting requested review from rfourquet

Assignees
No one assigned
Labels
domain:randomness Random number generation and the Random stdlib kind:minor change Marginal behavior change acceptable for a minor release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tpapp @StefanKarpinski @rfourquet @JeffBezanson @fredrikekre