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

DefaultDict accessor fails when the default is not the value type #901

Open
tpapp opened this issue Mar 25, 2024 · 4 comments
Open

DefaultDict accessor fails when the default is not the value type #901

tpapp opened this issue Mar 25, 2024 · 4 comments

Comments

@tpapp
Copy link

tpapp commented Mar 25, 2024

MWE:

pkg> st DataStructures
  [864edb3b] DataStructures v0.18.18

julia> using DataStructures

julia> dd = DefaultDict(1, :a => (2,))
DefaultDict{Symbol, Tuple{Int64}, Int64} with 1 entry:
  :a => (2,)

julia> dd[:b]
ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type Tuple{Int64}

Closest candidates are:
  convert(::Type{T}, ::T) where T<:Tuple
   @ Base essentials.jl:451
  convert(::Type{T}, ::T) where T
   @ Base Base.jl:84
  convert(::Type{T}, ::CartesianIndex) where T<:Tuple
   @ Base multidimensional.jl:136
  ...

Stacktrace:
 [1] get!
   @ Base ./dict.jl:481 [inlined]
 [2] get!
   @ Base ./abstractdict.jl:552 [inlined]
 [3] getindex
   @ DataStructures ~/.julia/packages/DataStructures/jFDPC/src/default_dict.jl:63 [inlined]
 [4] getindex(a::DefaultDict{Symbol, Tuple{Int64}, Int64}, args::Symbol)
   @ DataStructures ~/.julia/packages/DataStructures/jFDPC/src/delegate.jl:21
 [5] top-level scope
   @ REPL[111]:1
@tpapp
Copy link
Author

tpapp commented Mar 25, 2024

FWIW, I don't understand why it calls get!, and not get.

@oxinabox
Copy link
Member

oxinabox commented Mar 25, 2024

FWIW, I don't understand why it calls get!, and not get.

By calling get! and not get it means one can do things like:

dict_of_lists = DefaultDict(Vector)
for (key, item) in (:a=>1, :b=>1, :a=>10)
    push!(dict_of_lists[key], item)
end

Which is very useful -- in my experience this kinda pattern of a collection of collections is the most common use for a DefaultDict


I don't undestand there being heavy demand for if the default isn't the same type.
It would mean getindex is type unstable, which is problematic both from a code speed POV,
but also from like working out what operations are legal on it.

@tpapp
Copy link
Author

tpapp commented Mar 26, 2024

What I missed is that whenever access falls back to the default, it is stored in the parent dict, as in

julia> using DataStructures

julia> dd = DefaultDict(1, :a => 2)
DefaultDict{Symbol, Int64, Int64} with 1 entry:
  :a => 2

julia> dd[:b]
1

julia> dd
DefaultDict{Symbol, Int64, Int64} with 2 entries:
  :a => 2
  :b => 1

I find this design somewhat surprising, as it just replicates get!.

From the docs it was my expectation that getindex just falls back to the default without changing the parent.

@oxinabox
Copy link
Member

Yes, it replicates get! but with the ability to specify what to do at declaration time, rather than at access time.
I personally use get! on a regular Dict far more often than wanting to use a different type.
The alternative would be to replicate get, but as shown in my example above this cuts out a key use case.
We should improve its documentation

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

No branches or pull requests

2 participants