Skip to content

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

Merged
JeffBezanson merged 1 commit intoJuliaLang:masterfrom
tpapp:tp/randperm-narrow
Nov 29, 2018
Merged

Make randperm and randcycle use the argument's type.#29670
JeffBezanson merged 1 commit intoJuliaLang:masterfrom
tpapp:tp/randperm-narrow

Conversation

@tpapp
Copy link
Copy Markdown
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.

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 requested a review from rfourquet October 16, 2018 11:20
@fredrikekre fredrikekre added the randomness Random number generation and the Random stdlib label Oct 16, 2018
@StefanKarpinski
Copy link
Copy Markdown
Member

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

@StefanKarpinski StefanKarpinski added the minor change Marginal behavior change acceptable for a minor release label Oct 16, 2018
@JeffBezanson JeffBezanson added the 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 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
Copy Markdown
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 Dec 5, 2018
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
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
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

Labels

minor change Marginal behavior change acceptable for a minor release randomness Random number generation and the Random stdlib

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants