ready Accumulator for v1.0

src/accumulator.jl

@@ -33,9 +33,8 @@ function counter(seq)
 end
 
 eltype_for_accumulator(seq::T) where T = eltype(T)
-function eltype_for_accumulator(seq::Base.Generator)
-    Base.@default_eltype(seq)
-end
+eltype_for_accumulator(seq::Base.Generator) = Base.@default_eltype(seq)
+
 
 
 Base.copy(ct::Accumulator) = Accumulator(copy(ct.map))
@@ -44,9 +43,9 @@ Base.length(a::Accumulator) = length(a.map)
 
 ## retrieval
 
-Base.get(ct::Accumulator, x, default) = get(ct.map, x, default)
 # need to allow user specified default in order to
 # correctly implement "informal" AbstractDict interface
+Base.get(ct::Accumulator, x, default) = get(ct.map, x, default)
 
 Base.getindex(ct::Accumulator{T,V}, x) where {T,V} = get(ct.map, x, zero(V))
 
@@ -76,12 +75,11 @@ inc!(ct::Accumulator, x, v::Number) = (ct[x] += v)
 inc!(ct::Accumulator{T, V}, x) where {T, V} = inc!(ct, x, one(V))
 
 # inc! is preferred over push!, but we need to provide push! for the Bag interpreation
-# which is used by classified_collections.jl
 Base.push!(ct::Accumulator, x) = inc!(ct, x)
 Base.push!(ct::Accumulator, x, a::Number) = inc!(ct, x, a)
 
 # To remove ambiguities related to Accumulator now being a subtype of AbstractDict
-Base.push!(ct::Accumulator, x::Pair)  = inc!(ct, x)
+Base.push!(ct::Accumulator{P}, x::P) where P<:Pair  = inc!(ct, x)
 
 
 """
@@ -92,7 +90,9 @@ Decrements the count for `x` by `v` (defaulting to one)
 dec!(ct::Accumulator, x, v::Number) = (ct[x] -= v)
 dec!(ct::Accumulator{T,V}, x) where {T,V} = dec!(ct, x, one(V))
 
-#TODO: once we are done deprecating `pop!` for `reset!` then add `pop!` as an alias for `dec!`
+Base.pop!(ct::Accumulator, x) = dec!(ct, x)
+Base.pop!(ct::Accumulator, x, default) = haskey(ct, x) ? dec!(ct, x) : default
+
 
 """
     merge!(ct1::Accumulator, others...)
@@ -183,6 +183,11 @@ nsmallest(acc::Accumulator, n) = partialsort!(collect(acc), 1:n, by=last, rev=fa
 ###########################################################
 ## Multiset operations
 
+"""
+    MultiplicityException{K,V} <: Exception
+
+For errors related to havign a negetive count when using an Accumulator as a multiset.
+"""
 struct MultiplicityException{K, V} <: Exception
     k::K
     v::V

test/test_accumulator.jl

@@ -2,53 +2,72 @@
 
 @testset "Accumulators" begin
 
-    ct = counter(String)
+    @testset "Running tests" begin
+        #TODO: all these tests should be made independent.
+        # Currently they all need to run in a particular order as they share `ct`
+        ct = counter(String)
 
-    @testset "Core Functionality" begin
-        @assert isa(ct, Accumulator{String,Int})
+        @testset "Core Functionality" begin
+            @assert isa(ct, Accumulator{String,Int})
 
-        @test ct["abc"] == 0
-        @test !haskey(ct, "abc")
-        @test isempty(collect(keys(ct)))
-    end
+            @test ct["abc"] == 0
+            @test !haskey(ct, "abc")
+            @test isempty(collect(keys(ct)))
+        end
 
-    @testset "Test setindex!" begin
-        ct["b"] = 2
-        @test ct["b"] == 2
-        ct["b"] = 0
-        @test ct["b"] == 0
-    end
+        @testset "Test setindex!" begin
+            ct["b"] = 2
+            @test ct["b"] == 2
+            ct["b"] = 0
+            @test ct["b"] == 0
+        end
 
-    @testset "Test inc!" begin
-        inc!(ct, "a")
-        @test haskey(ct, "a")
-        @test ct["a"] == 1
+        @testset "Test inc!" begin
+            inc!(ct, "a")
+            @test haskey(ct, "a")
+            @test ct["a"] == 1
 
-        inc!(ct, "b", 2)
-        @test haskey(ct, "b")
-        @test ct["b"] == 2
-    end
+            inc!(ct, "b", 2)
+            @test haskey(ct, "b")
+            @test ct["b"] == 2
+        end
+
+        @testset "Test dec!" begin
+            dec!(ct, "b")
+            @test ct["b"] == 1
+            dec!(ct, "b", 16)
+            @test ct["b"] == -15
+            ct["b"] = 2
+        end
+
+        @testset "Test convert" begin
+            inc!(ct, "b", 0x3)
+            @test ct["b"] == 5
 
-    @testset "Test dec!" begin
-        dec!(ct, "b")
-        @test ct["b"] == 1
-        dec!(ct, "b", 16)
-        @test ct["b"] == -15
-        ct["b"] = 2
+            @test !haskey(ct, "abc")
+            @test ct["abc"] == 0
+
+            @test length(ct) == 2
+            @test length(collect(ct)) == 2
+            @test length(collect(keys(ct))) == 2
+            @test length(collect(values(ct))) == 2
+            @test sum(ct) == 6
+        end
     end
 
-    @testset "Test convert" begin
-        inc!(ct, "b", 0x3)
-        @test ct["b"] == 5
+    @testset "pop!" begin
+        acc = Accumulator(:a=>1, :b=>2, :c=>3)
+        @test pop!(acc, :c) == 2
+        @test acc == Accumulator(:a=>1, :b=>2, :c=>2)
 
-        @test !haskey(ct, "abc")
-        @test ct["abc"] == 0
+        # pop! with default differs from dec!
+        acc = Accumulator(:a=>1, :b=>2, :c=>3)
+        @test pop!(acc, :c, 3) == 2  # default, not decrement amount
+        @test acc == Accumulator(:a=>1, :b=>2, :c=>2)
 
-        @test length(ct) == 2
-        @test length(collect(ct)) == 2
-        @test length(collect(keys(ct))) == 2
-        @test length(collect(values(ct))) == 2
-        @test sum(ct) == 6
+        acc = Accumulator(:a=>1, :b=>2, :c=>3)
+        @test pop!(acc, :d, 4) == 4
+        @test acc == Accumulator(:a=>1, :b=>2, :c=>3)
     end
 
     @testset "From Pairs" begin 
@@ -71,11 +90,6 @@
         @test ct2["a"] == 3
         @test ct2["b"] == 2
         @test ct2["c"] == 2
-
-        merge!(ct, ct2)
-        @test ct["a"] == 4
-        @test ct["b"] == 7
-        @test ct["c"] == 2
     end
 
     @testset "From Dict" begin
@@ -260,8 +274,6 @@
             @test sprint(show,Accumulator(1 => 3)) == "Accumulator(1 => 3)"
             @test sprint(show,Accumulator(1 => 3, 3 => 4)) == "Accumulator(3 => 4, 1 => 3)"
         end
-
-
     end
 
 end # @testset Accumulators