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

Document Contains for Categoricals more clearly #534

Merged
merged 3 commits into from
Sep 26, 2023

Conversation

felixcremer
Copy link
Contributor

This would close #532
I am still wondering, whether we should give an error for Categorical{String} dimensions.

@rafaqz
Copy link
Owner

rafaqz commented Sep 6, 2023

Calling At also happens on points. The idea is that for values without an interval, containing a value us the same as being equal to it

@felixcremer
Copy link
Contributor Author

But I get an error when I run Contains on a Point dimension:

julia> arr = DimArray(rand(10,10,4), (X(1:10), Y(1:10), Dim{:Variable}(["root_moisture", "soil_moisture", "air_temperature", "something"])))
julia> arr[X=Contains(1)]
ERROR: ArgumentError: Points LookupArray cannot use `Contains`, use `Near` or `At` for Points.
Stacktrace:
  [1] contains(::DimensionalData.Dimensions.LookupArrays.Points, l::DimensionalData.Dimensions.LookupArrays.Sampled{Int64, UnitRange{Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Int64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.NoMetadata}, sel::Contains{Int64}; err::DimensionalData.Dimensions.LookupArrays._True)
    @ DimensionalData.Dimensions.LookupArrays ~/.julia/dev/DimensionalData/src/LookupArrays/selector.jl:299
  [2] contains(::DimensionalData.Dimensions.LookupArrays.Points, l::DimensionalData.Dimensions.LookupArrays.Sampled{Int64, UnitRange{Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Int64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.NoMetadata}, sel::Contains{Int64})
    @ DimensionalData.Dimensions.LookupArrays ~/.julia/dev/DimensionalData/src/LookupArrays/selector.jl:297
  [3] contains(l::DimensionalData.Dimensions.LookupArrays.Sampled{Int64, UnitRange{Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Int64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.NoMetadata}, sel::Contains{Int64}; kw::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ DimensionalData.Dimensions.LookupArrays ~/.julia/dev/DimensionalData/src/LookupArrays/selector.jl:291
  [4] contains(l::DimensionalData.Dimensions.LookupArrays.Sampled{Int64, UnitRange{Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Int64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.NoMetadata}, sel::Contains{Int64})
    @ DimensionalData.Dimensions.LookupArrays ~/.julia/dev/DimensionalData/src/LookupArrays/selector.jl:291
  [5] selectindices(l::DimensionalData.Dimensions.LookupArrays.Sampled{Int64, UnitRange{Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Int64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.NoMetadata}, sel::Contains{Int64}; kw::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ DimensionalData.Dimensions.LookupArrays ~/.julia/dev/DimensionalData/src/LookupArrays/selector.jl:283
  [6] selectindices(l::DimensionalData.Dimensions.LookupArrays.Sampled{Int64, UnitRange{Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Int64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.NoMetadata}, sel::Contains{Int64})
    @ DimensionalData.Dimensions.LookupArrays ~/.julia/dev/DimensionalData/src/LookupArrays/selector.jl:283
  [7] _dims2indices
    @ ~/.julia/dev/DimensionalData/src/Dimensions/indexing.jl:114 [inlined]
  [8] macro expansion
    @ ~/.julia/dev/DimensionalData/src/Dimensions/indexing.jl:56 [inlined]
  [9] _dims2indices(lookups::Tuple{DimensionalData.Dimensions.LookupArrays.Sampled{Int64, UnitRange{Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Int64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.NoMetadata}, DimensionalData.Dimensions.LookupArrays.Sampled{Int64, UnitRange{Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Int64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.NoMetadata}, DimensionalData.Dimensions.LookupArrays.Categorical{String, Vector{String}, DimensionalData.Dimensions.LookupArrays.Unordered, DimensionalData.Dimensions.LookupArrays.NoMetadata}}, dims::Tuple{X{DimensionalData.Dimensions.LookupArrays.Sampled{Int64, UnitRange{Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Int64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.NoMetadata}}, Y{DimensionalData.Dimensions.LookupArrays.Sampled{Int64, UnitRange{Int64}, DimensionalData.Dimensions.LookupArrays.ForwardOrdered, DimensionalData.Dimensions.LookupArrays.Regular{Int64}, DimensionalData.Dimensions.LookupArrays.Points, DimensionalData.Dimensions.LookupArrays.NoMetadata}}, Dim{:Variable, DimensionalData.Dimensions.LookupArrays.Categorical{String, Vector{String}, DimensionalData.Dimensions.LookupArrays.Unordered, DimensionalData.Dimensions.LookupArrays.NoMetadata}}}, I::Tuple{X{Contains{Int64}}, Nothing, Nothing})
    @ DimensionalData.Dimensions ~/.julia/dev/DimensionalData/src/Dimensions/indexing.jl:56
 [10] dims2indices
    @ ~/.julia/dev/DimensionalData/src/Dimensions/indexing.jl:51 [inlined]
 [11] dims2indices
    @ ~/.julia/dev/DimensionalData/src/Dimensions/indexing.jl:34 [inlined]
 [12] #getindex#39
    @ ~/.julia/dev/DimensionalData/src/array/indexing.jl:49 [inlined]
 [13] top-level scope
    @ REPL[36]:1

@rafaqz
Copy link
Owner

rafaqz commented Sep 6, 2023

Haha well thats totally inconsistent then, I think it has to do one or the other.

@rafaqz
Copy link
Owner

rafaqz commented Sep 9, 2023

Making this fail on Categorical is breaking, if we want that we should get it in on the current breaking release. Otherwise we should make this work on Points

@rafaqz rafaqz merged commit b4decde into rafaqz:main Sep 26, 2023
5 checks passed
@rafaqz rafaqz mentioned this pull request Sep 26, 2023
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

Successfully merging this pull request may close these issues.

Contains for categorical Dimensions should call Base.contains on the Strings
2 participants