Julia 0.6 rewrite #121

andyferris · 2017-03-21T04:42:15Z

This was quite a large amount of work. Most of the files have been rewritten, and hopefully code quality has improved a bit. Note these improvements:

Full multidimensional APL indexing should now be implemented.
Similarly, broadcast, broadcast!, map and map! now support arbitrary many static array inputs.
Universal use of the Size trait for dispatch
A Length trait and related pure functions
Reorganized files a little, tried to upgrade most code to the new v0.6 syntax and be more consistent with formatting and expression munging.
Started some work on static ranges - the eventual goal here is to upgrade from talking about sizes to talking about indices, so we can have static arrays with index ranges other than 1:n. (However, this wasn't exported.)

We have this breaking change:

similar_type defaults to SVector, etc - it does not preserve immutability. This helps e.g. with things like m[i, SVector(1,2,3)] where m::Matrix, and should generally be beneficial for speed.

We have this regression

Mixed static array - standard array operations like SMatrix * Vector have been removed. We can add these back in, if they are popular, but until we have inlined non-isbits immutables, we shouldn't do the simple thing here and wrap them in a SizedArray (well, we could, but it might cost more to allocate a pointer to the heap + gc, than to perform the actual operation).

I don't expect a full code review here. Nonetheless I'll leave this PR open for a day or two. (Please feel free to comment here before or after it is merged - we can always improve the code).

Closes #113 #109 #106 #95

rdeits · 2017-03-21T04:53:22Z

This is amazing. Thank you!

coveralls · 2017-03-21T05:08:14Z

Coverage increased (+0.6%) to 72.038% when pulling 0f3494b on julia-0.6 into 24a08f1 on master.

c42f

Epic effort!

I've only got down to indexing.jl so far, but wanted to send some comments through as it looks like I won't finish reading this today.

c42f · 2017-03-21T04:48:25Z

src/MMatrix.jl

-@inline ones{N,M}(::Type{MMatrix{N,M}}) = ones(MMatrix{N,M,Float64})
+#@inline eye{N,M}(::Type{MMatrix{N,M}}) = eye(MMatrix{N,M,Float64})
+#@inline zeros{N,M}(::Type{MMatrix{N,M}}) = zeros(MMatrix{N,M,Float64})
+#@inline ones{N,M}(::Type{MMatrix{N,M}}) = ones(MMatrix{N,M,Float64})


Are these obsolete, or forgotten? Ditto for the same thing in other files.

Yeah, probably should delete these.

What happened here is that I made the generic versions of these functions a bit more powerful.

c42f · 2017-03-21T04:51:52Z

src/SUnitRange.jl

+@pure Size(::SUnitRange{Start, End}) where {Start,End} = Size(End-Start+1)
+
+@pure @propagate_inbounds function getindex(x::SUnitRange{Start,End}, i::Int) where {Start, End}
+    @boundscheck if i < Start || i > End


I have a vague memory that @boundscheck wasn't reliable with a general expression. It does work with a function call as the expression.

Maybe worth looking into. Though I thought this pattern was used throughout Base.

It is, but I think it's usually written as

@boundscheck checkbounds(A, i)

or something. IIRC I had problems when the expression given to @boundscheck wasn't a function call.

So it looks like you've defined a static version of what I'm calling an IdentityRange or IdempotentRange, see JuliaArrays/IdentityRanges.jl#1 and JuliaLang/julia#21052. Just to check, you're aware that these are non-1 indices arrays you've defined here? If this is intentional, you should definitely define the indices function for them.

If that's not what you're intending, you should be checking against 1 <= i <= End - Start + 1 and then return i + Start - 1.

Haha thanks @timholy - these definitely are WIP. I think I'll split this off and put it on a branch for now.

These are just meant to be singleton array types, so that indices(SVector(1,2,3)) === SUnitRange{1,3}(). AFAICT IdentityRanges are meant to be a bit like a safe :? (How is it distinct from start:stop?)

I suppose that OneTo has been able to double-up for both purposes, for one-based arrays?

IdentityRanges are indeed a generalization of :, one that allows you to slice some or all of an axis while preserving the indices. As to how it differs from start:stop...if you're constructing a view v = view(a, s) with an AbstractVector s, the key identity is that v[i] == a[s[i]]. Since start:stop is a conventional UnitRange, it is indexed starting at 1, so v = view(a, start:stop) would be indexed starting at 1 no matter how a is indexed. In contrast, v = view(a, IdentityRange(start:stop)) would be indexed starting with start, no matter how a is indexed.

c42f · 2017-03-21T04:53:05Z

src/Scalar.jl

 end

-@inline (::Type{Scalar{T}}){T}(x::Tuple{T}) = Scalar{T}(x[1])
+@inline Scalar(x::Tuple{T}) where {T} = Scalar{T}(x[1])


What's this indexing x[1] about?

The abstract constructors will go Scalar(x) -> Scalar((x,)) (the internal functions also give a tuple). This then goes to Scalar{T}(x::T), which is an inner constructor (and is more specific than other constructors).

c42f · 2017-03-21T04:55:51Z

src/abstractarray.jl

-# mutable things stay mutable
-similar_type{SA<:Union{MVector,MMatrix,MArray},T,S}(::Type{SA},::Type{T},s::Size{S}) = mutable_similar_type(T,s,length_val(s))
+# should mutable things stay mutable?
+#similar_type{SA<:Union{MVector,MMatrix,MArray},T,S}(::Type{SA},::Type{T},s::Size{S}) = mutable_similar_type(T,s,length_val(s))


Is the behavior changed here?

Yes. I forgot to mention above. @timholy seemed supportive, so I left it this way.

c42f · 2017-03-21T04:57:05Z

src/abstractarray.jl

-        end
+@inline reshape(a::StaticArray, s::Size) = similar_type(a, s)(Tuple(a))
+@generated function reshape(a::AbstractArray, s::Size{S}) where {S}
+    if IndexStyle(a) == IndexLinear() # TODO this isn't "hyperpure"


But this is a TODO, because IndexStyle isn't really supported yet?

Actually, this one is critical. If a is defined after using StaticArrays, this will fail.

Can you make this safer by dispatching on the IndexStyle in "regular" functions?

reshape(a::AbstractArray, s::Size) = _reshape(IndexStyle(a), a, s) @generated function _reshape(::IndexLinear, a, s::Size{S}) where {S} ... end @generated function _reshape(::IndexStyle, a, s::Size{S}) where {S} ... end

c42f · 2017-03-21T06:26:01Z

src/broadcast.jl

+            end
+        end
+        j += 1
+    end


This loop looks exactly the same as in broadcast, with the exception of the dest[$j] = bit. Could be factored together?

c42f · 2017-03-21T06:27:35Z

src/convert.jl

+    end
+    return out
+end
+=#


What's all this commented out stuff? Speculative functionality? Removed stuff?

c42f · 2017-03-21T06:31:23Z

src/det.jl

    @inbounds return A[1]*A[4] - A[3]*A[2]
 end

-@inline function _det{T<:Unsigned}(::Size{(2,2)}, A::AbstractMatrix{T})
+@inline function _det(::Size{(2,2)}, A::StaticMatrix{T}) where {T <: Unsigned}


Can this where be avoided with StaticMatrix{<:Unsigned}?

Yes, it can

c42f · 2017-03-21T06:32:10Z

src/det.jl

        if istriu(A) || istril(A)
-            return convert($T2, det(UpperTriangular(A)))::$T2 # Is this a Julia bug that a convert is not type stable??
+            return convert(T2, det(UpperTriangular(A))) # Is this a Julia bug that a convert is not type stable??


The type assert was removed here. Obsolete comment?

ahh... I probably should test this...

c42f · 2017-03-21T06:32:55Z

src/eigen.jl

@@ -190,7 +193,7 @@ end
 # A small part of the code in the following method was inspired by works of David
 # Eberly, Geometric Tools LLC, in code released under the Boost Software
 # License (included at the end of this file).
-@inline function _eig{T<:Real}(::Size{(3,3)}, A::Base.LinAlg.RealHermSymComplexHerm{T}, permute, scale)
+@inline function _eig(::Size{(3,3)}, A::Base.LinAlg.RealHermSymComplexHerm{T}, permute, scale) where {T <: Real}


Can the where pieces be removed in this file and replaced with the shorthand?

This one uses T.

tkoolen · 2017-03-21T06:53:44Z

~~@rdeits, I think you broke Github.~~ Never mind, it was just me apparently; there were 4 copies of your comment with synchronized emojis between them.

andyferris · 2017-03-21T06:55:15Z

So it wasn't just me that saw that? I reloaded and it seems fine now.

timholy

You're a hero.

timholy · 2017-03-21T09:13:01Z

src/SUnitRange.jl

+@pure Size(::SUnitRange{Start, End}) where {Start,End} = Size(End-Start+1)
+
+@pure @propagate_inbounds function getindex(x::SUnitRange{Start,End}, i::Int) where {Start, End}
+    @boundscheck if i < Start || i > End


So it looks like you've defined a static version of what I'm calling an IdentityRange or IdempotentRange, see JuliaArrays/IdentityRanges.jl#1 and JuliaLang/julia#21052. Just to check, you're aware that these are non-1 indices arrays you've defined here? If this is intentional, you should definitely define the indices function for them.

If that's not what you're intending, you should be checking against 1 <= i <= End - Start + 1 and then return i + Start - 1.

timholy · 2017-03-21T09:20:37Z

src/abstractarray.jl

-        end
+@inline reshape(a::StaticArray, s::Size) = similar_type(a, s)(Tuple(a))
+@generated function reshape(a::AbstractArray, s::Size{S}) where {S}
+    if IndexStyle(a) == IndexLinear() # TODO this isn't "hyperpure"


Can you make this safer by dispatching on the IndexStyle in "regular" functions?

reshape(a::AbstractArray, s::Size) = _reshape(IndexStyle(a), a, s) @generated function _reshape(::IndexLinear, a, s::Size{S}) where {S} ... end @generated function _reshape(::IndexStyle, a, s::Size{S}) where {S} ... end

timholy · 2017-03-21T09:24:03Z

src/abstractarray.jl

+    if IndexStyle(a) == IndexLinear() # TODO this isn't "hyperpure"
+        exprs = [:(a[$i]) for i = 1:prod(S)]
+    else
+        exprs = [:(a[$(inds...)]) for inds ∈ CartesianRange(S)]


inds.I...? Also, does this imply that a[4,2] is defined for a SMatrix{3, 4}? Or do you mean to be iterating over Size(a)?

timholy · 2017-03-21T09:38:42Z

src/indexing.jl

-    S1, S2 = length(inds1), lengths(inds2)
-    exprs = [:(m[inds1[$j1], inds2[$j2]]) for j1 = 1:S1, j2 = 1:S2]
-    return Expr(:call, SMatrix{S1, S2, T}, Expr(:tuple, exprs...)) # I guess SMatrix should be fine?
+@propagate_inbounds function getindex(a::StaticArray, inds::Int...)


Do you really want to support inds::Int..., or would it be better to support only i::Vararg{Int, N} where N matches the dimensionality of the array? If you do that, then you get all the goodness of dropped 1s, etc, for free.

Also, you may not need this at all for IndexStyle(a)::IndexLinear.

Hmm... yes this seems like a valid point.

I should check if the implementation here is faster than a basic sub2ind one.

timholy · 2017-03-21T09:41:52Z

src/indexing.jl

-        return quote
-            $(Expr(:meta, :inline, :propagate_inbounds))
-            $(Expr(:call, :getindex, :a, Expr(:tuple, inds...)))
+@generated function _getindex_scalar(::Size{S}, a::StaticArray, inds::Int...) where S


If you need this, split this first segment into a sub2ind_expr function, or something?

timholy · 2017-03-21T09:46:17Z

src/indexing.jl

+    ind_types = inds.parameters
+    current_ind = ones(Int,length(ind_types))
+    more = true
+    while more


This overall logic seems repeated in many places...is it consolidatable?

That might have been better (but it's already written, so...)

coveralls · 2017-03-22T07:36:01Z

Coverage increased (+0.6%) to 71.964% when pulling 003c40f on julia-0.6 into 24a08f1 on master.

andyferris · 2017-03-22T23:38:14Z

@SimonDanisch - what is the plan for StaticArrays.FixedSizeArrays on Julia 0.6?

SimonDanisch · 2017-03-23T08:14:17Z

See if we can make it work?! ;) I will have to look into it very soon!

coveralls · 2017-03-23T23:43:07Z

Coverage increased (+0.6%) to 71.964% when pulling 13f2ff1 on julia-0.6 into 24a08f1 on master.

coveralls · 2017-03-24T03:51:01Z

Coverage increased (+0.6%) to 71.964% when pulling a0d6658 on julia-0.6 into 24a08f1 on master.

c42f

More partial review, got down to the end of mapreduce...

c42f · 2017-03-23T11:18:16Z

src/linalg.jl

+end
+@inline function _cross(::Size{(3,)}, a::StaticVector{<:Unsigned}, b::StaticVector{<:Unsigned})
+    T = typeof(a[2]*b[3]-a[3]*b[2])
+    @inbounds return similar_type(a, typeof(Signed(a[2]*b[3])-Signed(a[3]*b[2])))(((Signed(a[2]*b[3])-Signed(a[3]*b[2]), Signed(a[3]*b[1])-Signed(a[1]*b[3]), Signed(a[1]*b[2])-Signed(a[2]*b[1]))))


T unused here and above.

I guess someone wanted these Unsigned versions or they wouldn't be here... but they sure look like an oddity to me, and Base has no such specialization.

Yeah, it just happens to be needed for the determinant algorithm, and someone raised an issue about that! Seemed a bit silly but oh well.

c42f · 2017-03-23T11:27:34Z

src/linalg.jl

-        for j = 2:length(a)
-            expr = :($expr + conj(a[$j]) * b[$j])
-        end
+@inline dot(a::StaticVector, b::StaticVector) = _dot(same_size(a, b), a, b)


Could just call _vecdot()

c42f · 2017-03-23T11:28:07Z

src/linalg.jl

+    expr = :(conj(a[1]) * b[1])
+    for j = 2:prod(S)
+        expr = :($expr + conj(a[$j]) * b[$j])
+    end


Code golf :-)

expr = Expr(:call, :+, [:(conj(a[$j])*b[$j]) for j=1:prod(S)]...)

lol.

Are we sure that multi-argument + is performant?

c42f · 2017-03-23T11:31:13Z

src/linalg.jl

        return zero(real(eltype(a)))
    end

    expr = :(real(conj(a[1]) * a[1]))
-    for j = 2:length(a)
+    for j = 2:prod(S)
        expr = :($expr + real(conj(a[$j]) * a[$j]))


abs2(x) is probably better here than real(conj(x)*x)?

c42f · 2017-03-23T11:36:59Z

src/linalg.jl

        return zero(eltype(a))
    end

-    exprs = [:(a[$(sub2ind(s, i, i))]) for i = 1:s[1]]
+    exprs = [:(a[$(sub2ind(S, i, i))]) for i = 1:S[1]]
    total = reduce((ex1, ex2) -> :($ex1 + $ex2), exprs)


Not really this PR, but it's less Expr allocation/nesting to reduce this as total = Expr(:call, :+, exprs...).

That will wreck performance if exprs is too long because julia can't handle too many function argument well.

Although I have noticed this reduction pattern to be quite tough on inference and thus long compilation times.

Are we sure that multi-argument + is performant? (There will be a splat somewhere, which potentially ruins performance for vectors and matrices with 16+ elements)

I thought it was solved way back in 0.4 via a hack I spotted in inference.

But I've just done a test and you're right to be worried about this - generated code quality still falls off a cliff with more than 7 arguments to +. This is despite the fact that + is force inlined via a hack in inference.

Looking at @code_typed sumit(@SVector(ones(8))) of

@generated function sumit(v) s = Expr(:call, :+, [:(v[$i]) for i=1:length(v)]...) quote @inbounds s = $s s end end

shows some utterly strange things going on as a function of the SVector length.

c42f · 2017-03-23T11:38:18Z

src/linalg.jl

@@ -125,60 +79,56 @@ end
        @inbounds return $(Expr(:call, newtype, Expr(:tuple, exprs...)))
    end
 end
+=#


To delete? Will RowVector do the trick instead?

c42f · 2017-03-23T11:39:51Z

src/mapreduce.jl

+    if s === Size(a1)
+        return _same_size(s, as...)
+    else
+        throw(DimensionMismatch("Dimensions must match. Got inputs with $s and $(Size(a1))."))
    end
 end


Neato utility

c42f · 2017-03-23T11:43:34Z

src/mapreduce.jl

+        tmp = [:(a[$j][$i]) for j ∈ 1:length(a)]
+        exprs[i] = :(f($(tmp...)))
+    end
+    eltypes = [eltype(a[j]) for j ∈ 1:length(a)] # presumably, `eltype` is "hyperpure"?


julia-0.6 with all new hyperpure™ functionality, washing your generated functions even whiter.

lol.

Yes - should I make a PR to Julia NEWS.md highlighting our new hyperpure™ technology stack?

c42f

Well, I got to the bottom! Added some more comments on minor niceties, nothing big :-) There's a few things which have been removed and should be documented.

c42f · 2017-03-24T06:05:07Z

src/matrix_multiply.jl

-    can_mutate = !isbits(A) || !isbits(B) # !isbits implies can get a persistent pointer (to pass to BLAS). Probably will change to !isimmutable in a future version of Julia.
-    can_blas = T == TA && T == TB && T <: Union{Float64, Float32, Complex{Float64}, Complex{Float32}}
+@generated function _A_mul_B(Sa::Size{sa}, Sb::Size{sb}, a::StaticMatrix{Ta}, b::StaticMatrix{Tb}) where {sa, sb, Ta, Tb}
+    can_mutate = a.mutable && b.mutable # TODO this probably isn't safe. Maybe a trait??


This should be fine, shouldn't it? a and b are DataType in the generator context, and even if the field access here was turned into a call to getfield, it's an intrinsic (I think?) and can't be extended.

It's a bit funny... you can have a StaticArray struct which is immutable but for which setfield! works (such as SizedArray), and you can (in theory) have a mutable struct and not define setfield!. Annoying...

I probably should just specialize on Union{SizedMatrix{<:BlasFloat}, MMatrix{<:BlasFloat}}.

Looks good.

c42f · 2017-03-24T06:05:35Z

src/matrix_multiply.jl

+                @_inline_meta
+                T = promote_matprod(Ta, Tb)
+                C = similar(a, T, $S)
+                A_mul_B_blas!($S, C, Sa, Sb, a, b)


c42f · 2017-03-24T06:12:09Z

src/matrix_multiply.jl

-    TB = eltype(B)
-    T = promote_matprod(TA, TB)
-
-    can_blas = T == TA && T == TB && T <: Union{Float64, Float32, Complex{Float64}, Complex{Float32}}


👍 for less code!

c42f · 2017-03-24T06:12:46Z

src/solve.jl

+            (a[1,2]*a[3,1] - a[1,1]*a[3,2])*b[2] +
+            (a[1,1]*a[2,2] - a[1,2]*a[2,1])*b[3]) / d )
+end


It's a huge improvement to move these to traits. Nice!

c42f · 2017-03-24T06:14:33Z

src/traits.jl

-@pure getindex{S}(::Size{S}, i::Int) = i <= length(S) ? S[i] : 1
+struct Length{L}
+    function Length{L}() where L
+        check_length(L)


Oh, can you use that trick you were showing to avoid the need for check_length?

Not needed of course, just a nicety.

c42f · 2017-03-24T06:19:39Z

src/util.jl

@@ -28,6 +28,9 @@ end
    end
 end

+
+# TODO: the below seems to be type piracy...
+#=


Then let's just delete these, and document it?

Hard to know whether to depwarn them, really they should never have been in here in the first place.

Personally, I'd lean toward just deleting any type piracy, and documenting it in the release notes.

c42f · 2017-03-24T06:25:46Z

test/custom_types.jl

@@ -0,0 +1,7 @@
+@testset "Custom types" begin
+    # Issue 123
+    @eval (struct MyType{N, T} <: StaticVector{T}


You don't need the brackets around struct, I think?

c42f · 2017-03-24T06:30:00Z

test/linalg.jl

+        @test_broken @inferred(v1 + v4) === @SVector [6, 7, 8, 9]
+        @test_broken @inferred(v3 + v2) === @SVector [6, 7, 8, 9]
+        @test_broken @inferred(v1 - v4) === @SVector [-2, 1, 4, 7]
+        @test_broken @inferred(v3 - v2) === @SVector [-2, 1, 4, 7]


Could use a comment discussing what happened here. Same for other newly broken tests, I guess.

ie, obviously there's a decision to be made about whether we reimplement this functionality (in which case we delete the broken tests). Just a one liner about the decision to leave it broken for now, perhaps?

Well... probably a good idea...

andyferris · 2017-03-24T07:04:55Z

OK, I'll make a release to METADATA sometime over the next day or two - anyone who finds a bug before then, I'd appreciate a report.

coveralls · 2017-03-24T07:07:23Z

Coverage decreased (-1.4%) to 70.034% when pulling 5ebaf5c on julia-0.6 into 24a08f1 on master.

dpsanders · 2017-03-24T13:58:41Z

Just wanted to mention that ValidatedNumerics's tests pass with this branch on 0.6 -- many thanks for the hard work!

timholy · 2017-03-25T20:32:33Z

I'm delighted to report that ImageFiltering passes as well! Thanks so very much!

I assume the new release will be called 0.4?

andyferris · 2017-03-25T21:37:46Z

Yep, to make it official, I just tagged v0.4.0 in github - its up to attobot now :)

andyferris · 2017-03-25T21:41:50Z

JuliaLang/METADATA.jl#8494

tkelman · 2017-03-26T09:20:53Z

src/mapreduce.jl

-            @inbounds return $expr
-        end
-    end
+@inline function mapreduce(f, op, a::StaticArray...)


Little odd, but this allows for mapreduce(f, op) which would normally error. same_size() isn't defined right now, but it might at some point, by accident or piracy.

This will still error :)

I do see your point (though it seems unlikely that this signature would clash with ones defined by others). Are you requesting that I modify the signature (to tag the release in METADATA)?

same_size() isn't defined right now, but it might at some point, by accident or piracy.

I'm not quite sure how to respond, since I'm not even exporting this. I do intend to keep watch on new features and breaking changes for Julia 0.7/1.0 (and a new export from Base would count as more than bugfix w.r.t. semver). But yes, it would be useful to generally have a function like this exported from Base one day.

Just trying to think of ways in which the 0-occurrences version of this might be a problem (or not). I guess there could also be other packages that might do the same basic signature but for vararg of their own type, in which case mapreduce(f, op) might be ambiguous between here and that other package? Maybe a little safer to have the signature 1-or-more instead of 0-or-more, but maybe I'm just being paranoid.

tkelman · 2017-03-26T09:35:51Z

REQUIRE

@@ -1,2 +1 @@
 julia 0.5
-Compat 0.19.0


@compat is still used in test/fixed_size_arrays.jl, though it looks like that's now dead code

Yes, that file is not deleted because I think Simon Danisch will revive it sometime

foreachfield and staticschema cleanup

Andy Ferris and others added 5 commits March 21, 2017 14:27

WIP v0.6 support

bff627a

More WIP for Julia v0.6

a1eab92

Fix minor bug in matrix multiply

4773d4c

Finished v0.6 fixes (hopefully!)

28f405a

Updated travis, appveyer

ed6fb3b

Enable code coverage of v0.6

0f3494b

andyferris mentioned this pull request Mar 21, 2017

Code rewrite for Julia v0.6 #113

Closed

26 tasks

c42f reviewed Mar 21, 2017

View reviewed changes

timholy reviewed Mar 21, 2017

View reviewed changes

Code review fixes

003c40f

Fixed #123, inner constructor ambiguous with error message constructor

a0d6658

andyferris force-pushed the julia-0.6 branch from 13f2ff1 to a0d6658 Compare March 24, 2017 03:41

c42f reviewed Mar 24, 2017

View reviewed changes

c42f approved these changes Mar 24, 2017

View reviewed changes

More code review fixes

5ebaf5c

andyferris merged commit b935326 into master Mar 24, 2017

andyferris deleted the julia-0.6 branch March 24, 2017 07:03

This was referenced Mar 24, 2017

World-age failure in M * v with Colors #109

Closed

incorrect pure annotations #95

Closed

andyferris mentioned this pull request Mar 24, 2017

Dispatch issues with size() on v0.6-dev #106

Closed

tkelman reviewed Mar 26, 2017

View reviewed changes

c42f mentioned this pull request Apr 1, 2017

SVector + Vector: Julia 0.5 vs 0.6 #131

Closed

oschulz pushed a commit to oschulz/StaticArrays.jl that referenced this pull request Apr 4, 2023

Merge pull request JuliaArrays#121 from JuliaArrays/pv/static

4483b57

foreachfield and staticschema cleanup

Julia 0.6 rewrite #121

Julia 0.6 rewrite #121

Conversation

andyferris commented Mar 21, 2017 • edited Loading

rdeits commented Mar 21, 2017

coveralls commented Mar 21, 2017

c42f left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyferris Mar 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkoolen commented Mar 21, 2017 • edited Loading

andyferris commented Mar 21, 2017

timholy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Mar 22, 2017

andyferris commented Mar 22, 2017

SimonDanisch commented Mar 23, 2017

coveralls commented Mar 23, 2017

coveralls commented Mar 24, 2017

c42f left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC Mar 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

c42f left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyferris commented Mar 21, 2017 •

edited

Loading

andyferris Mar 21, 2017 •

edited

Loading

tkoolen commented Mar 21, 2017 •

edited

Loading

KristofferC Mar 24, 2017 •

edited

Loading