Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for MapAlgebra.sumByKey with a Unit value #325

Closed
wants to merge 1 commit into from

Conversation

jcoveney
Copy link
Contributor

@jcoveney jcoveney commented Jul 3, 2014

This is the cause of the error in twitter/scalding#935

@jcoveney
Copy link
Contributor Author

jcoveney commented Jul 3, 2014

This has a potentially deeper ramification for scalding: this means that we can't use MapAlgebra, becasue we could be throwing away rows (if they are equal to the zero)

@@ -120,6 +120,11 @@ object CollectionSpecification extends Properties("Collections") {
!MapAlgebra.removeZeros(m).values.toSet.contains(0)
}

property("MapAlgebra.sumByKey with Unit works") = forAll { l: Set[String] =>
val lst = l.toList
l == MapAlgebra.sumByKey((lst ++ lst ++ lst).map((_, ()))).keys
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to use MapEquiv here. () is the unit of this type, so this is not a failing test case. This is the expected behavior.

I would expect that you may get Map.empty here.

@jcoveney
Copy link
Contributor Author

jcoveney commented Jul 3, 2014

I'm going to close here and just remove the use of MapAlgebra in scalding, since sparse summing isn't what we want

@jcoveney jcoveney closed this Jul 3, 2014
@jcoveney jcoveney deleted the jco/map_algebra_sum_by_key branch July 3, 2014 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants