Skip to content

adding some matrix extensions used by etsy #365

Merged
merged 4 commits into from Apr 8, 2013

2 participants

@jattenberg

Etsy uses scalding for social network analysis and graph processing. Here are some of the changes we've made to scalding to make this work. Mostly additional functionality.

@johnynek
johnynek commented Apr 1, 2013

Super awesome.

Can I talk you into adding some tests here:
https://github.com/twitter/scalding/blob/develop/scalding-core/src/test/scala/com/twitter/scalding/mathematics/MatrixTest.scala

We've found that between hadoop and cascading tests are super valuable to catch even things that look like they would work fine. Also, as we upgrade cascading and hadoop, minor flow planning issues have caused subtle failures that we only caught with good test coverage.

Thanks very much for doing this.

@jattenberg

np.

@jattenberg

Updated, all tests pass.

@johnynek johnynek and 1 other commented on an outdated diff Apr 8, 2013
...n/scala/com/twitter/scalding/mathematics/Matrix.scala
@@ -573,6 +626,40 @@ class Matrix[RowT, ColT, ValT]
new Matrix[RowT,ColT,(ValT,ValU)](rowSym, colSym, valSym, zipped, sizeHint + that.sizeHint)
}
+ /**
+ * removes any elements in this matrix that also appear in the argument matrix
+ */
+ def filterBy[ValU](that : Matrix[RowT,ColT,ValU]) : Matrix[RowT,ColT,ValT] = {
@johnynek
johnynek added a note Apr 8, 2013

I'm worried that in scala, filter takes a predicate that means keep the items (and there is filterNot for the opposite). It seem like this method is somewhat the reverse semantics, if the predicate here is: that is defined at (row, col), then this is a filterNot.

Can we be more verbose here? removeElementsBy? Then we could also have keepElementsBy and keep with your keepBy semantics below.

@jattenberg
jattenberg added a note Apr 8, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@johnynek
johnynek commented Apr 8, 2013

Awesome! Thanks.

@johnynek johnynek merged commit ec6e256 into twitter:develop Apr 8, 2013

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.