search

KristofferC / PGFPlotsX.jl

Plots in Julia using the PGFPlots LaTeX package
Other
301 stars 40 forks source link

Add push! and append! for options. #183

Closed tpapp closed 4 years ago

tpapp commented 4 years ago

Addesses #182, NEEDS DOCS.

codecov-io commented 4 years ago

Codecov Report

Merging #183 into master will increase coverage by 0.46%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #183      +/-   ##
==========================================
+ Coverage   87.31%   87.77%   +0.46%     
==========================================
  Files           9        9              
  Lines         536      540       +4     
==========================================
+ Hits          468      474       +6     
+ Misses         68       66       -2
Impacted Files Coverage Δ
src/options.jl 86.66% <100%> (+4.52%) :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 77122f5...79b24ae. Read the comment docs.

KristofferC commented 4 years ago

Does these really make sense? Options is similar to a dict which does not have push! or append!. A Dict has setindex! and merge!.

fredrikekre commented 4 years ago 
julia> d = Dict("hello"=>"world")
Dict{String,String} with 1 entry:
  "hello" => "world"

julia> push!(d, "foo"=>"bar")
Dict{String,String} with 2 entries:
  "hello" => "world"
  "foo"   => "bar"
KristofferC commented 4 years ago

Lol, alright then. How about append!?

tpapp commented 4 years ago

There is no append! for Dict in Base, but that may may just be an oversight. I generally tend to think of append! as the vector form of push!, which is why I included it.

But I can of course remove it from this PR, it is not crucial.

KristofferC commented 4 years ago

Might as well keep it.