JuliaCollections / JuliaCollections/DataStructures.jl
Propose changes to the interface for default value to `DefaultDict`
Nobody has claimed this yet.
- Dominant language
- Julia
- Stars
- 745
- Forks
- 261
- PR merge metrics
- No merged PRs in 30d
Description
Currently, an object is passed by reference to initialize the default for DefaultDict. There are 2 alternative interfaces I think:
- construct from the object, e.g. via
deepcopy; - remove usage of the object in the interface, which enforces passing in of a constructor function (like Python's implementation).
I would like to take the opportunity to hear from us on the pros and cons of the options. I derived the following myself and might very likely meet with some blindspot. Let me apologize if I ended up wasting our time in the end 🙇
analysis of current interface
Ok so let's go. In my experience, I would like to claim that the current interface is "easy to do the wrong thing".
cannot reliably get a constructed default
Let me start with the well-known example.
Say I want to define a DefaultDict{Int, Vector{Int}} with an empty vector. We can do every thing that Python can, e.g. via DefaultDict{Int, Vector{Int}}(Vector{Int}), or DefaultDict{Int, Vector{Int}}(() -> []), and so on. So it's easy to do the right thing.
However, we can also pass in an object. Raise hand ✋ if this has happened to you unwittingly before:
julia> dd = DefaultDict{Int, Vector{Int}}([1])
DefaultDict{Int64, Vector{Int64}, Vector{Int64}}()
julia> push!(dd[0], 2)
2-element Vector{Int64}:
1
2
julia> dd[1]
2-element Vector{Int64}:
1
2
From time to time I will make this mistake, e.g. with vectors within vectors as default because I don't use it often enough to remember the pain. Looking at the occasional issues submitted (e.g. #149, #542, discourse 29440), I'm not alone in this.
cannot reliably get a passed by reference default
If there's no use case for the mutable default, then I think we can close the case already. So let's assume the contrary.
Then, it is still easy to do the wrong thing! For example, this time we have:
julia> dd = DefaultDict{Int, Vector{Int}}([])
DefaultDict{Int64, Vector{Int64}, Vector{Any}}()
julia> push!(dd[0], 2)
1-element Vector{Int64}:
2
julia> dd[1]
Int64[]
The reason lies deep within the implementation. Because when we use []::Vector{Any} to create the default empty Vector{Int}, there's a type mismatch. So regardless of whether a get! or setindex! is used, the underlying Dict inside dd.d.d will call convert (source).
It is thus very hard to one-take our code and reliably determine how the default object affects the initialized default values. To me, an interface should be more intuitive than that.
proposal to construct the default value via deepcopy
So I would like to propose for us to consider option (1.): construct the default value via deepcopy.
Firstly I can see why having an object interface is useful, when the object is a bit-type, or contain immutable data that we do not want to waste time copying. For that I do not think we want to go the Python way and remove this interface.
However is there a use case for a mutable shared default that's why that interface is supported? For me there do exist times when mapping all keys to the same values by default is a smart hack, but they are seldom deal-breaking for me.
There is an increase in interface consistency. For normal users, they can use the rule of thumb that the default will always be distinct objects.
The strongest con I can come up with is a runtime slowdown. Every object will be initialized with a call to deepcopy. I would argue that most beginner users ain't thinking about milking the performance yet. Also for most data structures used as default, their deepcopy are not that much more expensive than other means like constructors or convert. The more advanced user will know to either call the function form to avoid the deep copying if constructing is cheaper, or dispatch on Base.deepcopy_internal if they want to reuse some immutable data within the default object.
call to action
So as mentioned, I do believe there's already much thought that went into the current implementation so pardon me if I went off-tangent. Would appreciate if I could be pointed in the right direction in that case. Otherwise if what I said does make some sense, I would also hope that we can give the proposed change a thought.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing DefaultDict's current constructors and the linked base/dict.jl conversion behavior. Compare the deepcopy and constructor-function proposals against the mutable and shared-default examples in the issue. Done means reaching a settled interface decision and documenting how distinct versus shared defaults behave.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- julia
- Domain
- data
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100