From 891a6083e5256695502d66bc0a674362b97e5e38 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Sun, 11 Jun 2017 03:20:46 -0500 Subject: [PATCH] RFC: Re-enable axis checks in constructor This is a major breaking change. Back before this package was officially registered, these checks were dropped. That means that many of the indexing methods that require sorted vectors are now broken. Enforcing these preconditions in the constructor is, unfortunately, terribly breaking. I think the alternative here would be to delay the checks until indexing occurs, but the downside there is that for non-range axes, it would require a `issorted` pass through the data upon every operation. We could defer to the SortedVector type more, but that easily walks us into a type-unstable API. Thoughts? --- src/core.jl | 4 ++++ test/core.jl | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/core.jl b/src/core.jl index 3b93930..aa34651 100644 --- a/src/core.jl +++ b/src/core.jl @@ -205,6 +205,7 @@ AxisArray(A::AbstractArray, vects::Tuple{Vararg{Union{AbstractVector, Axis}}}) = function AxisArray{T,N}(A::AbstractArray{T,N}, axs::NTuple{N,Axis}) checksizes(axs, _size(A)) || throw(ArgumentError("the length of each axis must match the corresponding size of data")) checknames(axisnames(axs...)...) + checkaxes(axs...) AxisArray{T,N,typeof(A),typeof(axs)}(A, axs) end @@ -552,6 +553,9 @@ function checkaxis(::Type{Categorical}, ax) end end +checkaxes() = nothing +@inline checkaxes(ax, axs...) = (checkaxis(ax); checkaxes(axs...)) + _length(A::AbstractArray) = length(linearindices(A)) _length(A) = length(A) _size(A::AbstractArray) = map(length, indices(A)) diff --git a/test/core.jl b/test/core.jl index 3e80a26..31db8d5 100644 --- a/test/core.jl +++ b/test/core.jl @@ -145,6 +145,16 @@ A = @inferred(AxisArray(reshape(1:24, 2,3,4), Axis{:y}(1//10:1//10:3//10), Axis{:z}(["a", "b", "c", "d"]))) +# Ensure Axes conform to requirements +@test_throws ArgumentError AxisArray(1:3, [1,2,1]) +@test_throws ArgumentError AxisArray(1:3, -3:-1:1) +@test_throws ArgumentError AxisArray(1:3, [:a,:b,:a]) +@test_throws ArgumentError A[[2,1], :, :] +@test_throws ArgumentError A[:, [1,2,1], :] +@test_throws ArgumentError A[:, :, [1, 1, 2]] +@test A[1, 1, ["a", "b"]] == [1, 7] +@test_broken A[1, 1, ["b", "a"]] == [7, 1] + # Test axisdim @test axisdim(A, Axis{:x}) == axisdim(A, Axis{:x}()) == 1 @test axisdim(A, Axis{:y}) == axisdim(A, Axis{:y}()) == 2