-
Notifications
You must be signed in to change notification settings - Fork 391
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
Improved multi pick list map vectorizer test #292
Improved multi pick list map vectorizer test #292
Conversation
…ultiPickListMapVectorizerTest
…ultiPickListMapVectorizerTest
@@ -94,8 +94,7 @@ final class MultiPickListMapVectorizerModel[T <: OPMap[Set[String]]] private[op] | |||
operationName: String, | |||
uid: String | |||
)(implicit tti: TypeTag[T]) | |||
extends SequenceModel[T, OPVector](operationName = operationName, uid = uid) | |||
with VectorizerDefaults with CleanTextMapFun { | |||
extends SequenceModel[T, OPVector](operationName = operationName, uid = uid) with CleanTextMapFun { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wsuchy please also remove VectorizerDefaults
mixin from MultiPickListMapVectorizer
class
@@ -46,11 +46,20 @@ import org.slf4j.LoggerFactory | |||
|
|||
|
|||
@RunWith(classOf[JUnitRunner]) | |||
class MultiPickListMapVectorizerTest extends FlatSpec with TestSparkContext with AttributeAsserts { | |||
class MultiPickListMapVectorizerTest extends | |||
OpEstimatorSpec[OPVector, SequenceModel[MultiPickListMap, OPVector], MultiPickListMapVectorizer[MultiPickListMap]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better put the newline before the extends
class MultiPickListMapVectorizerTest
extends OpEstimatorSpec[...] {
}
@@ -70,7 +79,8 @@ class MultiPickListMapVectorizerTest extends FlatSpec with TestSparkContext with | |||
lazy val (dataSetAllEmpty, _) = TestFeatureBuilder(top.name, | |||
Seq(MultiPickListMap.empty, MultiPickListMap.empty, MultiPickListMap.empty)) | |||
|
|||
val vectorizer = new MultiPickListMapVectorizer().setCleanKeys(true).setMinSupport(0).setTopK(10).setInput(top, bot) | |||
val estimator:MultiPickListMapVectorizer[MultiPickListMap] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idents:
val estimator: MultiPickListMapVectorizer[MultiPickListMap] =
new MultiPickListMapVectorizer()...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, see comments
Codecov Report
@@ Coverage Diff @@
## master #292 +/- ##
======================================
Coverage 86.4% 86.4%
======================================
Files 319 319
Lines 10452 10452
Branches 346 346
======================================
Hits 9031 9031
Misses 1421 1421
Continue to review full report at Codecov.
|
..and another test fixed