Add `copy` definition for `Dictionary` that works with `undef`

[mtfishman]

Add copy definition for Dictionary that works with undef. Also add some tests for operations on Dictionary with undef elements.

@andyferris, let me know if you have another way you would prefer to define this. The previous definition forwarded to copyto!, which forwarded to map!, which threw and error if the elements of the Dictionary are undef:

julia> dict = Dictionary{Int, String}([1, 2], undef)
2-element Dictionary{Int64, String}
 1 │ #undef
 2 │ #undef

julia> copy(dict)
ERROR: UndefRefError: access to undefined reference
Stacktrace:
 [1] getindex
   @ ./array.jl:861 [inlined]
 [2] gettokenvalue
   @ ~/.julia/dev/Dictionaries/src/Dictionary.jl:308 [inlined]
 [3] map!(f::typeof(identity), out::Dictionary{Int64, String}, d::Dictionary{Int64, String})
   @ Dictionaries ~/.julia/packages/Dictionaries/sMIv4/src/map.jl:57
 [4] copyto!
   @ ~/.julia/dev/Dictionaries/src/AbstractDictionary.jl:493 [inlined]
 [5] copy
   @ ~/.julia/dev/Dictionaries/src/AbstractDictionary.jl:488 [inlined]
 [6] copy(dict::Dictionary{Int64, String})
   @ Dictionaries ~/.julia/packages/Dictionaries/sMIv4/src/AbstractDictionary.jl:485
 [7] top-level scope
   @ REPL[7]:1

The advantage of the previous approach is that it could work for a generic AbstractDictionary, which is still available as a generic fallback.

[andyferris]

I made a start with doing the same for ArrayDictionary (because I don't want it to feel like a second-class citizen) but there's still a bit more to do there.

[mtfishman]

Sounds good, thanks for adding the ArrayDictionary definition.

It looks like one of the ArrayDictionary tests is failing since it is hitting another undef bug (one related to #78 and #83).

[mtfishman]

@andyferris I fixed the ArrayDictionary test by adding a new ArrayDictionary constructor similar to the one you added for Dictionary in #83, let me know if there is anything left to do here.

I wasn't quite sure how the copying behavior should work, currently it uses convert so sometimes uses a reference to the ArrayDictionary values that are input, are you following a certain convention for constructors and copy/view behavior?

[mtfishman]

I guess Base.Array constructors never use references to the original data:

julia> x = randn(4)
4-element Vector{Float64}:
  1.600142496761151
 -0.6903694806276901
  0.057400450235790795
 -0.3882590806579015

julia> y = Vector{Float64}(x)
4-element Vector{Float64}:
  1.600142496761151
 -0.6903694806276901
  0.057400450235790795
 -0.3882590806579015

julia> x[1] = 1.0
1.0

julia> y
4-element Vector{Float64}:
  1.600142496761151
 -0.6903694806276901
  0.057400450235790795
 -0.3882590806579015

Perhaps we should follow that convention here and I should use Vector{T}(indexable.values) instead of convert(Vector{T}, indexable.values).

[codecov[bot]]

Codecov Report

Merging #85 (0693d10) into master (eded5db) will increase coverage by 1.08%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
+ Coverage   76.08%   77.16%   +1.08%     
==========================================
  Files          18       18              
  Lines        1990     1988       -2     
==========================================
+ Hits         1514     1534      +20     
+ Misses        476      454      -22     

Impacted Files	Coverage Δ
src/ArrayDictionary.jl	84.84% <100.00%> (+3.59%)	:arrow_up:
src/Dictionary.jl	79.89% <100.00%> (+5.71%)	:arrow_up:
src/show.jl	93.82% <0.00%> (+0.56%)	:arrow_up:
src/tokens.jl	24.24% <0.00%> (+0.71%)	:arrow_up:
src/indexing.jl	69.64% <0.00%> (+9.64%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update eded5db...0693d10. Read the comment docs.