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

Specialize map(T, ::AbstractRange) for more range types to preserve the result as a range #40746

Closed
wants to merge 3 commits into from
Closed

Specialize map(T, ::AbstractRange) for more range types to preserve the result as a range #40746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6816000
Broadcasting ranges may preserve types
jishnub May 7, 2021
0886343
Update base/abstractarray.jl
jishnub May 7, 2021
1d71d87
do not implement broadcast
jishnub May 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions base/abstractarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1152,12 +1152,30 @@ isempty(a::AbstractArray) = (length(a) == 0)

## range conversions ##

map(::Type{T}, r::AbstractRange{T}) where {T} = r
map(::Type{T}, r::StepRange) where {T<:Real} = T(r.start):T(r.step):T(last(r))
map(::Type{T}, r::OneTo) where {T<:Integer} = oneto(T(last(r)))
map(::Type{Bool}, r::OneTo) = map(Bool, UnitRange(r))
map(::Type{T}, r::OneTo) where {T<:Real} = map(T, UnitRange(r))
map(::Type{T}, r::UnitRange) where {T<:Real} = T(r.start):T(last(r))
map(::Type{T}, r::StepRangeLen) where {T<:AbstractFloat} = convert(StepRangeLen{T}, r)
function map(::Type{T}, r::LinRange) where T<:AbstractFloat
LinRange(T(r.start), T(r.stop), length(r))
end
# Special conversions for IdentityUnitRange
# Since IdentityUnitRange is defined as <: AbstractUnitRange{Int},
# its eltype may not be converted in Base,
# nor is it possible in general to map a type over it preserving its axes.
# We define the `Int` case here so that OffsetArrays may define the conversion
# for other eltypes through type-piracy.
# In the future this type-piracy may not be necessary if IdentityUnitRange is
# defined to be <: AbstractUnitRange{<:Integer}
map(::Type{Int}, r::IdentityUnitRange) = r
# For IdentityUnitRange{<:OneTo}, the map may be evaluated correctly in Base
# We pop the parent range as the result may not always be representable as an IdentityUnitRange
map(::Type{T}, r::IdentityUnitRange{<:OneTo}) where {T<:Real} = map(T, axes1(r))
# this method is for ambiguity resolution
map(::Type{Int}, r::IdentityUnitRange{<:OneTo}) = r
Comment on lines +1173 to +1178
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

For r::IdentityUnitRange, given that IdentityUnitRange(r.indices) === r, we could possibly use the deconstruct-and-then-construct alternative to solve ambiguity:

Suggested change
map(::Type{Int}, r::IdentityUnitRange) = r
# For IdentityUnitRange{<:OneTo}, the map may be evaluated correctly in Base
# We pop the parent range as the result may not always be representable as an IdentityUnitRange
map(::Type{T}, r::IdentityUnitRange{<:OneTo}) where {T<:Real} = map(T, axes1(r))
# this method is for ambiguity resolution
map(::Type{Int}, r::IdentityUnitRange{<:OneTo}) = r
Base.map(::Type{T}, r::IdentityUnitRange) where {T<:Real} = identity_or_map(T, axes1(r))
identity_or_map(::Type{Int}, r::AbstractUnitRange) = IdentityUnitRange(r) # the caller `map` thus becomes `identity`
identity_or_map(::Type{T}, r::AbstractUnitRange) = map(T, r)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will stack overflow as axes1(r) === r for Base.IdentityUnitRange if the parent range is not a Base.OneTo

julia> r = Base.IdentityUnitRange(2:3)
Base.IdentityUnitRange(2:3)

julia> map(Int32, r)
ERROR: StackOverflowError:


## unsafe/pointer conversions ##

Expand Down
1 change: 1 addition & 0 deletions base/multidimensional.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ module IteratorsMD
CartesianIndex(index::NTuple{N,Integer}) where {N} = CartesianIndex{N}(index)
CartesianIndex(index::Integer...) = CartesianIndex(index)
CartesianIndex{N}(index::Vararg{Integer,N}) where {N} = CartesianIndex{N}(index)
CartesianIndex{N}(I::CartesianIndex{N}) where {N} = I
# Allow passing tuples smaller than N
CartesianIndex{N}(index::Tuple) where {N} = CartesianIndex{N}(fill_to_length(index, 1, Val(N)))
CartesianIndex{N}(index::Integer...) where {N} = CartesianIndex{N}(index)
Expand Down
59 changes: 59 additions & 0 deletions test/ranges.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1889,3 +1889,62 @@ end
@test_throws BoundsError r[true:true:false]
@test_throws BoundsError r[true:true:true]
end

@testset "map a type over a range" begin
for r = Any[1:5, Base.OneTo(3), UnitRange(1.0, 4.0), 1:1:5, Base.IdentityUnitRange(Base.OneTo(3))]
for T in [Int8, Int16, Int32, Int64, Int128, BigInt]
r2 = map(T, r)
if r isa AbstractUnitRange
@test r2 isa AbstractUnitRange
else
@test r2 isa AbstractRange
end
@test r2 == r
end
for T in [Float16, Float32, Float64, BigFloat]
r2 = map(T, r)
@test r2 isa AbstractRange
@test r2 == r
end
end

r = Base.IdentityUnitRange(2:4)
r2 = map(Int, r)
@test r2 isa AbstractUnitRange
@test eltype(r) == Int
@test r2 == r

for r in Any[LinRange{Int}(1, 4, 4), LinRange(1, 4, 4), range(float(1), stop = float(4), step = float(1))]
for T in [Float32, Float64, BigFloat]
r2 = map(T, r)
@test r2 isa AbstractRange
@test r2 == r
end
end

@testset "Bool" begin
for fn = 0:1
for r in Any[1:fn, Base.OneTo(fn), 1:1:1, range(float(1), stop = float(fn), step = float(1)),
LinRange{Int}(1, fn, fn), LinRange(1, fn, fn), Base.IdentityUnitRange(Base.OneTo(fn))]
r2 = map(Bool, r)
@test eltype(r2) == Bool
@test r2 == r
@test r2 == map(Bool, r)
end
end
for fn = 0:1
for r in Any[0:fn, 0:1:fn, range(float(0), stop = float(fn), step = float(1)),
LinRange{Int}(0, fn, fn + 1), LinRange(0, fn, fn + 1)]
r2 = map(Bool, r)
@test eltype(r2) == Bool
@test r2 == r
@test r2 == map(Bool, r)
end
end
end
# test with a type that is not a subtype of Number
R = range(CartesianIndex(1), step = CartesianIndex(1), length = 4)
@test map(CartesianIndex, 1:4) == CartesianIndices((1:4,)) == R
R = range(CartesianIndex(1,1), step = CartesianIndex(2,2), length = 4)
@test map(CartesianIndex, 1:2:7, 1:2:7) == R
end