-
Notifications
You must be signed in to change notification settings - Fork 397
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/upgrade string indexing tests #294
Conversation
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
@tovbinm, The signature of the transformer spec is:
The transformer passed must inherit from OpPipelineStage, Transformer, and OpTransformer. OpTransformerWrapper is missing the OpTransformer piece. |
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
Ha! True. Its actually on purpose :) ok! So we need a new base class for testing wrapped stages then. |
Related issues
Improve test coverage for transformers & estimators #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