Closed crupley closed 5 years ago
@crupley can you elaborate what's not working with the base specs and wrapper stages? (cc @snabar)
@tovbinm, The signature of the transformer spec is:
abstract class OpTransformerSpec[O <: FeatureType : WeakTypeTag : ClassTag,
TransformerType <: OpPipelineStage[O] with Transformer with OpTransformer : ClassTag]
The transformer passed must inherit from OpPipelineStage, Transformer, and OpTransformer. OpTransformerWrapper is missing the OpTransformer piece.
Merging #294 into master will increase coverage by
<.01%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## master #294 +/- ##
==========================================
+ Coverage 86.4% 86.41% +<.01%
==========================================
Files 319 319
Lines 10452 10452
Branches 346 346
==========================================
+ Hits 9031 9032 +1
+ Misses 1421 1420 -1
Impacted Files | Coverage Δ | |
---|---|---|
...ce/op/stages/impl/feature/OpOneHotVectorizer.scala | 96.77% <ø> (ø) |
:arrow_up: |
...scala/com/salesforce/op/test/OpEstimatorSpec.scala | 93.33% <ø> (ø) |
:arrow_up: |
...p/stages/impl/feature/TextMapPivotVectorizer.scala | 100% <ø> (ø) |
:arrow_up: |
...ages/impl/feature/MultiPickListMapVectorizer.scala | 100% <ø> (ø) |
:arrow_up: |
...force/op/stages/impl/feature/OpIndexToString.scala | 100% <0%> (+50%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 44ecb7a...17f7381. Read the comment docs.
Ha! True. Its actually on purpose :) ok! So we need a new base class for testing wrapped stages then.
@crupley here you go - https://github.com/salesforce/TransmogrifAI/pull/295
Related issues Improve test coverage for transformers & estimators https://github.com/salesforce/TransmogrifAI/issues/278
Describe the proposed solution
OpStringIndexer
andOpStringIndexerNoFilter
OpStringIndexerNoFilterTest
OpIndexToStringNoFilterTest
Describe alternatives you've considered
Attempted to upgrade
OpStringIndexerTest
andOpIndexToStringTest
but these both extend an wrapper-type class that is not compatible with the new test spec.Additional context None