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

setindex! introducing a constraint isn't always desirable #73

Open
glennmoy opened this issue May 17, 2022 · 0 comments
Open

setindex! introducing a constraint isn't always desirable #73

glennmoy opened this issue May 17, 2022 · 0 comments

Comments

@glennmoy
Copy link
Member

This came up in an internal implementation.

When we introduce a new element to a KeyedDataset the setindex! method assigns a constraint if one isn't already associated with it's dimpath.

AxisSets.jl/src/indexing.jl

Lines 122 to 130 in eb33c8c

for d in dimnames(val)
dimpath = (key..., d)
# Similar to construction, if our dimpath isn't present in any existing constraints
# then we introduce a new one that's `Pattern(:__, dim)`
if all(c -> !in(dimpath, c), ds.constraints)
push!(ds.constraints, Pattern(:__, d))
end
end

However, this isn't necessarily desirable as the new element might share a fieldname with an existing element and introduce and so introduce a conflicting constraint.

MWE

julia> using AxisSets, AxisKeys
julia> using AxisSets: Pattern

# assume the following
julia> ds = KeyedDataset(
           (:train, :input) => KeyedArray(ones(5, 5); time=1:5, id='a':'e'),
           (:predict, :input) => KeyedArray(ones(5); id='a':'e'),
           (:train, :output) => KeyedArray(ones(5); time=1:5);
           constraints=Pattern[
               (:train, :_, :time),
               (:predict, :_, :time),
               (:__, :input, :id),  # offending constraint
           ]
       )

# assign a new element with `:id` dimname - introduces `(:__, :id)` constraint
julia> ds[(:train, :weights)] = KeyedArray(ones(5), id='a':'e')

julia> ds.constraints
OrderedCollections.OrderedSet{Pattern} with 4 elements:
  Pattern((:train, :_, :time))
  Pattern((:predict, :_, :time))
  Pattern((:train, :input, :id))
  Pattern((:__, :id))

julia> ds
Error showing value of type KeyedDataset:
ERROR: ArgumentError: Collection has multiple elements, must contain exactly 1 element
Stacktrace:
  [1] only
    @ ./iterators.jl:1327 [inlined]
  [2] _only(x::Vector{Int64})
    @ AxisSets ~/.julia/packages/AxisSets/27klG/src/AxisSets.jl:27
  [3] (::AxisSets.var"#20#22"{Vector{Pattern}, Tuple{Symbol, Symbol}})(dimname::Symbol)
    @ AxisSets ~/.julia/packages/AxisSets/27klG/src/dataset.jl:97
  [4] map(f::AxisSets.var"#20#22"{Vector{Pattern}, Tuple{Symbol, Symbol}}, t::Tuple{Symbol, Symbol})
    @ Base ./tuple.jl:214
  [5] show(io::IOContext{Base.TTY}, ds::KeyedDataset)
    @ AxisSets ~/.julia/packages/AxisSets/27klG/src/dataset.jl:95
   ...

In this instance one would be better off redefining the offending constraint as (:__, :id) but this might not always be possible/desirable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant