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

Updated TextMapPivotVectorizerTest to use OpEstimatorSpec #290

Merged
merged 3 commits into from Apr 17, 2019

Conversation

tovbinm
Copy link
Collaborator

@tovbinm tovbinm commented Apr 17, 2019

Related issues

TextMapPivotVectorizerTest is not using OpEstimatorSpec

Describe the proposed solution

  1. Updated TextMapPivotVectorizerTest to use OpEstimatorSpec
  2. Remove VectorizerDefaults mixin from TextMapPivotVectorizer and TextMapPivotVectorizerModel

@tovbinm tovbinm changed the title Mt/map pivot vec test Updated TextMapPivotVectorizerTest to use OpEstimatorSpec Apr 17, 2019
@tovbinm tovbinm requested a review from wsuchy April 17, 2019 17:32
Copy link
Contributor

@wsuchy wsuchy left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Apr 17, 2019

Codecov Report

Merging #290 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #290      +/-   ##
==========================================
+ Coverage    86.4%   86.42%   +0.01%     
==========================================
  Files         319      319              
  Lines       10453    10452       -1     
  Branches      351      346       -5     
==========================================
+ Hits         9032     9033       +1     
+ Misses       1421     1419       -2
Impacted Files Coverage Δ
...scala/com/salesforce/op/test/OpEstimatorSpec.scala 93.33% <ø> (-0.22%) ⬇️
...p/stages/impl/feature/TextMapPivotVectorizer.scala 100% <ø> (ø) ⬆️
...es/src/main/scala/com/salesforce/op/OpParams.scala 89.79% <0%> (+4.08%) ⬆️

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 1b3edfe...e87d23d. Read the comment docs.

@tovbinm tovbinm merged commit 462226a into master Apr 17, 2019
@tovbinm tovbinm deleted the mt/map-pivot-vec-test branch April 17, 2019 19:45
@Jauntbox Jauntbox mentioned this pull request May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants