Period addition is not associative

[stevengj]

julia> p1 = Dates.Second(2) + (Dates.Millisecond(7) + Dates.Millisecond(1))
2 seconds, 8 milliseconds

julia> p2 = (Dates.Second(2) + Dates.Millisecond(7)) + Dates.Millisecond(1)
2 seconds, 7 milliseconds, 1 millisecond

julia> p2 == p1
false

Seems like the problem could be fixed by making the CompoundPeriod constructor canonicalize its contents by combining periods of the same type.

[stevengj]

Also not commutative:

julia> Dates.Millisecond(1) + (Dates.Second(2) + Dates.Millisecond(7))
ERROR: `+` has no method matching +(::Millisecond, ::CompoundPeriod)

[quinnj]

Ok, I have the commutative fix.

Yeah, we should probably compress the periods as they're added to CompoundPeriod so there are no duplicates.

[stevengj]

Also not that == is just using object equality for CompoundPeriod, which is not correct.

[stevengj]

Seems like this should do it in the constructor:

type CompoundPeriod
    periods::Array{Period,1}
    function CompoundPeriod(p::Vector{Period})
        n = length(p)
        n < 2 && return new(p)
        sort!(p, rev=true, lt=periodisless)
        # canonicalize p by merging equal period types                          
        i = j = 1
        while j <= n
            k = j+1
            while k <= n
                if typeof(p[j]) == typeof(p[k])
                    p[j] += p[k]
                    k += 1
                else
                    break
                end
            end
            p[i] = p[j]
            i += 1
            j = k
        end
        return new(resize!(p, i-1))
    end
end

[stevengj]

Correction: should also canonicalize to eliminate zeros.

[stevengj]

I'll submit a PR shortly

[stevengj]

Shouldn't CompoundPeriod be a subtype of AbstractTime (like Period)?

[stevengj]

Also notice that (+)(x::CompoundPeriod,y::Period) and (-)(x::CompoundPeriod,y::Period) currently mutate x (via push!), which seems undesirable.