Skip to content

Commit

Permalink
Revert to an error for getindex
Browse files Browse the repository at this point in the history
  • Loading branch information
odow committed Mar 23, 2023
1 parent f105fe6 commit dbfdca7
Show file tree
Hide file tree
Showing 16 changed files with 124 additions and 438 deletions.
163 changes: 30 additions & 133 deletions docs/src/manual/containers.md
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,22 @@ julia> DataFrames.DataFrame(table)
4 │ 2 B (2, :B)
```

### Keyword indexing

If all axes are named, you can use keyword indexing:

```jldoctest containers_dense
julia> x[i = 2, j = :A]
(2, :A)
julia> x[i = :, j = :B]
1-dimensional DenseAxisArray{Tuple{Int64, Symbol},1,...} with index sets:
Dimension 1, Base.OneTo(2)
And data, a 2-element Vector{Tuple{Int64, Symbol}}:
(1, :B)
(2, :B)
```

## SparseAxisArray

A [`Containers.SparseAxisArray`](@ref) is created when the index sets are
Expand Down Expand Up @@ -352,6 +368,20 @@ julia> DataFrames.DataFrame(table)
4 │ 3 A (3, :A)
```

### Keyword indexing

If all axes are named, you can use keyword indexing:

```jldoctest containers_sparse
julia> x[i = 2, j = :A]
(2, :A)
julia> x[i = :, j = :B]
JuMP.Containers.SparseAxisArray{Tuple{Int64, Symbol}, 1, Tuple{Int64}} with 2 entries:
[2] = (2, :B)
[3] = (3, :B)
```

## Forcing the container type

Pass `container = T` to use `T` as the container. For example:
Expand Down Expand Up @@ -485,136 +515,3 @@ JuMP.Containers.SparseAxisArray{Int64, 2, Tuple{Int64, Int64}} with 2 entries:
[1, 2] = 3
[1, 4] = 5
```

## Keyword indexing

JuMP v1.10.0 added experimental support for keyword indexing of
[`Containers.DenseAxisArray`](@ref) and [`Containers.SparseAxisArray`](@ref).
Keyword indexing lets you uses the named indices of [`Containers.DenseAxisArray`](@ref)
and [`Containers.SparseAxisArray`](@ref) as keyword arguments when indexing a
container.

### The container macro

For containers constructed using the [`Containers.@container`](@ref) macro,
opt-in to the keyword indexing syntax by passing
`enable_keyword_indexing = true`:

```jldoctest
julia> Containers.@container(
x[i=2:4, j=1:3],
i + j,
enable_keyword_indexing = true,
)
2-dimensional DenseAxisArray{Int64,2,...} with index sets:
Dimension 1, 2:4
Dimension 2, Base.OneTo(3)
And data, a 3×3 Matrix{Int64}:
3 4 5
4 5 6
5 6 7
julia> x[i = 2, j = 1]
3
julia> Containers.@container(
y[i=2:4, j=1:3; i > j],
i + j,
enable_keyword_indexing = true,
)
JuMP.Containers.SparseAxisArray{Int64, 2, Tuple{Int64, Int64}} with 6 entries:
[2, 1] = 3
[3, 1] = 4
[3, 2] = 5
[4, 1] = 5
[4, 2] = 6
[4, 3] = 7
julia> y[i = 2:4, j = 1]
JuMP.Containers.SparseAxisArray{Int64, 1, Tuple{Int64}} with 3 entries:
[2] = 3
[3] = 4
[4] = 5
```

### The JuMP macros

For containers constructed using the JuMP macros like [`@variable`](@ref),
opt-in using [`enable_keyword_indexing`](@ref):

```jldoctest
julia> using JuMP
julia> model = Model();
julia> @variable(model, x[i = 2:3])
1-dimensional DenseAxisArray{VariableRef,1,...} with index sets:
Dimension 1, 2:3
And data, a 2-element Vector{VariableRef}:
x[2]
x[3]
julia> x[i = 2]
ERROR: Keyword indexing is disabled. To enable, pass `enable_keyword_indexing = true` to the `Containers.@container` macro, or call `JuMP.enable_keyword_indexing(model, true)` before calling any JuMP macros like `@variable`.
Stacktrace:
[...]
julia> model = Model();
julia> enable_keyword_indexing(model, true)
julia> @variable(model, x[i = 2:3])
1-dimensional DenseAxisArray{VariableRef,1,...} with index sets:
Dimension 1, 2:3
And data, a 2-element Vector{VariableRef}:
x[2]
x[3]
julia> x[i = 2]
x[2]
```

### Limitations

The keyword indexing syntax is currently opt-in because it does not work for
`Array`:

```jldoctest
julia> Containers.@container(
x[i=1:4, j=1:3],
i + j,
enable_keyword_indexing = true,
)
ERROR: Keyword indexing is not supported with Array.
Stacktrace:
[...]
```

Work-around this limitation by forcing the container type:

```jldoctest
julia> model = Model();
julia> enable_keyword_indexing(model, true)
julia> @variable(model, x[i = 1:3])
3-element Vector{VariableRef}:
x[1]
x[2]
x[3]
julia> x[i = 2]
ERROR: MethodError: no method matching getindex(::Vector{VariableRef}; i=2)
[...]
julia> @variable(model, y[i = 1:3], container = DenseAxisArray)
1-dimensional DenseAxisArray{VariableRef,1,...} with index sets:
Dimension 1, Base.OneTo(3)
And data, a 3-element Vector{VariableRef}:
y[1]
y[2]
y[3]
julia> y[i = 2]
y[2]
```
1 change: 0 additions & 1 deletion docs/src/reference/models.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ object_dictionary
unregister
latex_formulation
set_string_names_on_creation
enable_keyword_indexing
```

## Working with attributes
Expand Down
11 changes: 0 additions & 11 deletions src/Containers/Containers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,6 @@ Base.IndexStyle(::IndexAnyCartesian, ::IndexAnyCartesian) = IndexAnyCartesian()

export DenseAxisArray, SparseAxisArray

_check_keyword_indexing_allowed(::Any) = nothing

function _check_keyword_indexing_allowed(::Nothing)
return error(
"Keyword indexing is disabled. To enable, pass " *
"`enable_keyword_indexing = true` to the `Containers.@container` " *
"macro, or call `JuMP.enable_keyword_indexing(model, true)` " *
"before calling any JuMP macros like `@variable`.",
)
end

include("DenseAxisArray.jl")
include("SparseAxisArray.jl")

Expand Down
11 changes: 3 additions & 8 deletions src/Containers/DenseAxisArray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ struct DenseAxisArray{T,N,Ax,L<:NTuple{N,_AxisLookup}} <: AbstractArray{T,N}
data::Array{T,N}
axes::Ax
lookup::L
names::Union{Nothing,NTuple{N,Symbol}}
names::NTuple{N,Symbol}
end

function Base.Array{T,N}(x::DenseAxisArray) where {T,N}
Expand Down Expand Up @@ -208,6 +208,7 @@ function DenseAxisArray(
) where {T,N}
@assert length(axes) == N
new_axes = _abstract_vector.(axes) # Force all axes to be AbstractVector!
names = something(names, ntuple(n -> Symbol("#$n"), N))
return DenseAxisArray(data, new_axes, build_lookup.(new_axes), names)
end

Expand Down Expand Up @@ -355,7 +356,6 @@ _is_range(::Any) = false
_is_range(::Union{Vector{Int},Colon}) = true

function _kwargs_to_args(A::DenseAxisArray{T,N}; kwargs...) where {T,N}
_check_keyword_indexing_allowed(A.names)
return ntuple(N) do i
kw = keys(kwargs)[i]
if A.names[i] != kw
Expand All @@ -381,11 +381,7 @@ function Base.getindex(A::DenseAxisArray{T,N}, args...; kwargs...) where {T,N}
return A.data[new_indices...]::T
end
new_axes = _getindex_recurse(A.axes, new_indices, _is_range)
names = if A.names === nothing
nothing
else
A.names[findall(_is_range, new_indices)]
end
names = A.names[findall(_is_range, new_indices)]
return DenseAxisArray(A.data[new_indices...], new_axes...; names = names)
end

Expand Down Expand Up @@ -747,7 +743,6 @@ function _fixed_indices(view_axes::Tuple, axes::Tuple)
end

function _kwargs_to_args(A::DenseAxisArrayView{T,N}; kwargs...) where {T,N}
_check_keyword_indexing_allowed(A.data.names)
non_default_indices = _fixed_indices(A.axes, A.data.axes)
return ntuple(N) do i
kw = keys(kwargs)[i]
Expand Down
7 changes: 3 additions & 4 deletions src/Containers/SparseAxisArray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ julia> array[:b, 3]
"""
struct SparseAxisArray{T,N,K<:NTuple{N,Any}} <: AbstractArray{T,N}
data::Dict{K,T}
names::Union{Nothing,NTuple{N,Symbol}}
names::NTuple{N,Symbol}
end

function SparseAxisArray(d::Dict{K,T}) where {T,N,K<:NTuple{N,Any}}
return SparseAxisArray(d, nothing)
return SparseAxisArray(d, ntuple(n -> Symbol("#$n"), N))
end

Base.length(sa::SparseAxisArray) = length(sa.data)
Expand Down Expand Up @@ -99,7 +99,6 @@ function Base.setindex!(
end

function _kwargs_to_args(d::SparseAxisArray{T,N}; kwargs...) where {T,N}
_check_keyword_indexing_allowed(d.names)
if length(kwargs) != N
throw(BoundsError(d, kwargs))
end
Expand Down Expand Up @@ -284,7 +283,7 @@ function Base.copy(
# type information to call SparseAxisArray(dict). As a work-around, we
# explicitly construct the type of the resulting SparseAxisArray.
# For more, see JuMP issue #2867.
return SparseAxisArray{Any,N,K}(dict, nothing)
return SparseAxisArray{Any,N,K}(dict, ntuple(n -> Symbol("#$n"), N))
end
return SparseAxisArray(dict)
end
Expand Down
26 changes: 0 additions & 26 deletions src/Containers/container.jl
Original file line number Diff line number Diff line change
Expand Up @@ -71,36 +71,10 @@ function container(f::Function, indices, D, names)
return container(f, indices, D)
end

struct _ForceEnableKeywordIndexing{T}
names::T
end

function container(f::Function, indices, ::Type{AutoContainerType}, names)
return container(f, indices, default_container(indices), names)
end

function container(
f::Function,
indices,
::Type{AutoContainerType},
names::_ForceEnableKeywordIndexing,
)
return container(f, indices, default_container(indices), names)
end

function container(f::Function, indices, D, names::_ForceEnableKeywordIndexing)
return container(f, indices, D, names.names)
end

function container(
::Function,
::Any,
::Type{Array},
::_ForceEnableKeywordIndexing,
)
return error("Keyword indexing is not supported with Array.")
end

function container(f::Function, indices)
return container(f, indices, default_container(indices))
end
Expand Down
35 changes: 6 additions & 29 deletions src/Containers/macro.jl
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,7 @@ end
index_vars::Vector{Any},
indices::Expr,
code,
requested_container::Union{Symbol,Expr};
keyword_indexing::Union{Bool,Expr},
requested_container::Union{Symbol,Expr},
)
Used in macros to construct a call to [`container`](@ref). This should be used
Expand Down Expand Up @@ -289,8 +288,7 @@ function container_code(
index_vars::Vector{Any},
indices::Expr,
code,
requested_container::Union{Symbol,Expr};
keyword_indexing::Union{Bool,Symbol,Expr} = false,
requested_container::Union{Symbol,Expr},
)
if isempty(index_vars)
return code
Expand All @@ -310,19 +308,9 @@ function container_code(
else
# This is a symbol or expression from outside JuMP, so we need to escape
# it.
keyword_indexing = missing
esc(requested_container)
end
return quote
names = if $keyword_indexing === missing
$index_vars
elseif $keyword_indexing === true
$_ForceEnableKeywordIndexing($index_vars)
else
nothing
end
$container($f, $indices, $container_type, names)
end
return Expr(:call, container, f, indices, container_type, index_vars)
end

"""
Expand Down Expand Up @@ -383,23 +371,12 @@ JuMP.Containers.SparseAxisArray{Int64,2,Tuple{Int64,Int64}} with 6 entries:
```
"""
macro container(args...)
args, kwargs, requested_container = _extract_kw_args(args)
args, kw_args, requested_container = _extract_kw_args(args)
@assert length(args) == 2
enable_keyword_indexing = false
for kw in kwargs
@assert Meta.isexpr(kw, :(=), 2)
@assert kw.args[1] == :enable_keyword_indexing
enable_keyword_indexing = esc(kw.args[2])
end
@assert isempty(kw_args)
var, value = args
index_vars, indices = build_ref_sets(error, var)
code = container_code(
index_vars,
indices,
esc(value),
requested_container;
keyword_indexing = enable_keyword_indexing,
)
code = container_code(index_vars, indices, esc(value), requested_container)
if isexpr(var, :vect) || isexpr(var, :vcat)
return code
else
Expand Down
Loading

0 comments on commit dbfdca7

Please sign in to comment.