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.

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

Modify documentation for view to take into account range special case #38536

Merged
merged 7 commits into from
Nov 30, 2020

Conversation

dlfivefifty
Copy link
Contributor

view sometimes does not return a SubArray, e.g.,

julia> view(2:5, 2:3) # returns a range as type is immutable
3:4

This is not really consistent with the docstring for view. So this PR modifies the docstring.

Cf JuliaArrays/FillArrays.jl#84 where we want to do something similar with FillArrays.jl but want to make sure it's justified.

Copy link
Member

@tkf tkf left a comment

Choose a reason for hiding this comment

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

LGTM. I added a suggestion but I don't think that's a hard requirement.

base/subarray.jl Outdated Show resolved Hide resolved
Co-authored-by: Takafumi Arakaki <takafumi.a@gmail.com>
@mcabbott
Copy link
Contributor

Looks like this changed in 1.5, for anyone else wondering. The PR was #26872.

julia> view(1:10, 3:4)
2-element view(::UnitRange{Int64}, 3:4) with eltype Int64:
 3
 4

julia> VERSION
v"1.4.2"

Copy link
Member

@mbauman mbauman left a comment

Choose a reason for hiding this comment

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

Ok, I got carried away. I've never been thrilled with defining view() as taking a view.

This is a suggested wholesale reworking of the text; please see what you think.

base/subarray.jl Outdated Show resolved Hide resolved
base/subarray.jl Outdated Show resolved Hide resolved
base/subarray.jl Outdated Show resolved Hide resolved
dlfivefifty and others added 2 commits November 25, 2020 18:36
Co-authored-by: Matt Bauman <mbauman@gmail.com>
Co-authored-by: Matt Bauman <mbauman@gmail.com>
@mbauman mbauman added the docs This change adds or pertains to documentation label Nov 25, 2020
Co-authored-by: Matt Bauman <mbauman@gmail.com>
@mbauman mbauman requested a review from tkf November 25, 2020 18:41
base/subarray.jl Outdated Show resolved Hide resolved
@tkf tkf requested a review from mbauman November 25, 2020 21:24
@tkf tkf unassigned mbauman Nov 25, 2020
dlfivefifty and others added 2 commits November 25, 2020 23:00
Co-authored-by: Takafumi Arakaki <takafumi.a@gmail.com>
@JeffBezanson JeffBezanson merged commit 056be22 into JuliaLang:master Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants