Merge pull request #95 from JuliaArrays/teh/disable_convert_error

For now, disable the distinction between coercion and conversion

Author: timholy

READMEOLD.md

@@ -106,3 +106,13 @@ julia> oa2[ax2[2]]
 ```
 
 `IdOffsetRange`s apply the offset both to the values and the indices of the range, and otherwise preserve the parent range.
+
+!!! warning
+
+    There are circumstances where constructing a specific type of `IdOffsetRange` cannot be supported without changing the axes of the range (see [`OffsetArrays.IdOffsetRange`](@ref).)
+    In the future, this package will distinguish between *construction*  and *conversion*:
+
+    - construction (aka, *coercion*) will always succeed, even if it has to change the axes of the result (Examples: `RangeType(rng)`, `typeof(rng1)(rng2)`)
+    - conversion will succeed only if it can preserve both the values and the axes (Examples: `convert(RangeType, rng)`, `oftype(rng1, rng2)`)
+
+    While these behave equivalently now (conversion currently performs coercion), developers are encouraged to "future-proof" their code by choosing the behavior appropriate for each usage.

docs/src/internals.md

@@ -53,22 +53,25 @@ has `r[3] == 3` and `r[4] == 4`, and `r[1]` would throw a `BoundsError`.
 In this latter case, a shift in the axes was needed because `Base.OneTo` ranges
 must start with value 1.
 
-In contrast, *conversion* preserves both the values and the indices, throwing an error
-when this is not achievable. For instance,
+!!! warning
 
-    r = convert(OffsetArrays.IdOffsetRange{Int,UnitRange{Int}}, 3:4)
+    In the future, *conversion* will preserve both the values and
+    the indices, throwing an error when this is not achievable. For instance,
 
-has `r[1] == 3` and `r[2] == 4` and would satisfy `r == 3:4`, whereas
+        r = convert(OffsetArrays.IdOffsetRange{Int,UnitRange{Int}}, 3:4)
 
-```jldoctest; setup=:(import OffsetArrays)
-julia> convert(OffsetArrays.IdOffsetRange{Int,Base.OneTo{Int}}, 3:4)
-ERROR: ArgumentError: first element must be 1, got 3
-```
+    has `r[1] == 3` and `r[2] == 4` and would satisfy `r == 3:4`, whereas
+
+    ```
+    julia> convert(OffsetArrays.IdOffsetRange{Int,Base.OneTo{Int}}, 3:4)    # future behavior, not present behavior
+    ERROR: ArgumentError: first element must be 1, got 3
+    ```
 
-where the error arises because the result could not have the same axes as the input.
+    where the error will arise because the result could not have the same axes as the input.
 
-An important corollary is that `typeof(r1)(r2)` and `oftype(r1, r2)` behave differently:
-the first coerces `r2` to be of the type of `r1`, whereas the second converts.
+    An important corollary is that `typeof(r1)(r2)` and `oftype(r1, r2)` will behave differently:
+    the first coerces `r2` to be of the type of `r1`, whereas the second converts.
+    Developers are urged to future-proof their code by choosing the behavior appropriate for each usage.
 """
 struct IdOffsetRange{T<:Integer,I<:AbstractUnitRange{T}} <: AbstractUnitRange{T}
     parent::I
@@ -100,13 +103,14 @@ function IdOffsetRange{T}(r::IdOffsetRange) where T<:Integer
 end
 IdOffsetRange(r::IdOffsetRange) = r
 
-# Conversion preserves both the values and the indexes, throwing an InexactError if this
-# is not possible.
-Base.convert(::Type{IdOffsetRange{T,I}}, r::IdOffsetRange{T,I}) where {T<:Integer,I<:AbstractUnitRange{T}} = r
-Base.convert(::Type{IdOffsetRange{T,I}}, r::IdOffsetRange) where {T<:Integer,I<:AbstractUnitRange{T}} =
-    IdOffsetRange{T,I}(convert(I, r.parent), r.offset)
-Base.convert(::Type{IdOffsetRange{T,I}}, r::AbstractUnitRange) where {T<:Integer,I<:AbstractUnitRange{T}} =
-    IdOffsetRange{T,I}(convert(I, r), 0)
+# TODO: uncomment these when Julia is ready
+# # Conversion preserves both the values and the indexes, throwing an InexactError if this
+# # is not possible.
+# Base.convert(::Type{IdOffsetRange{T,I}}, r::IdOffsetRange{T,I}) where {T<:Integer,I<:AbstractUnitRange{T}} = r
+# Base.convert(::Type{IdOffsetRange{T,I}}, r::IdOffsetRange) where {T<:Integer,I<:AbstractUnitRange{T}} =
+#     IdOffsetRange{T,I}(convert(I, r.parent), r.offset)
+# Base.convert(::Type{IdOffsetRange{T,I}}, r::AbstractUnitRange) where {T<:Integer,I<:AbstractUnitRange{T}} =
+#     IdOffsetRange{T,I}(convert(I, r), 0)
 
 offset_coerce(::Type{Base.OneTo{T}}, r::Base.OneTo) where T<:Integer = convert(Base.OneTo{T}, r), 0
 function offset_coerce(::Type{Base.OneTo{T}}, r::AbstractUnitRange) where T<:Integer

src/axes.jl

@@ -63,10 +63,14 @@ using CatIndices: BidirectionalVector
     @test typeof(r2) === typeof(rs)
     @test same_value(r2, 1:3)
     check_indexed_by(r2, 1:3)
-    @test_throws ArgumentError oftype(ro, rs)
+    # These two broken tests can be fixed by uncommenting the `convert` definitions
+    # in axes.jl, but unfortunately Julia may not quite be ready for this. (E.g. `reinterpretarray.jl`)
+    @test_broken try oftype(ro, rs); false catch err true end  # replace with line below
+    # @test_throws ArgumentError oftype(ro, rs)
     @test @inferred(oftype(ro, Base.OneTo(2))) === OffsetArrays.IdOffsetRange(Base.OneTo(2))
     @test @inferred(oftype(ro, 1:2)) === OffsetArrays.IdOffsetRange(Base.OneTo(2))
-    @test_throws ArgumentError oftype(ro, 3:4)
+    @test_broken try oftype(ro, 3:4); false catch err true end
+    # @test_throws ArgumentError oftype(ro, 3:4)
 end
 
 @testset "Single-entry arrays in dims 0:5" begin