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

Fixed onSetInput so is always called with new input #280

Merged
merged 15 commits into from
Apr 12, 2019
Merged

Conversation

leahmcguire
Copy link
Collaborator

Related issues
Metadata for transformers was not being updated when raw feature filter removed features

Describe the proposed solution
Moved onSetInput call into the base (internal) set input call

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context about the changes here.

@codecov
Copy link

codecov bot commented Apr 11, 2019

Codecov Report

Merging #280 into master will increase coverage by 0.01%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #280      +/-   ##
=========================================
+ Coverage   86.39%   86.4%   +0.01%     
=========================================
  Files         319     319              
  Lines       10450   10453       +3     
  Branches      354     561     +207     
=========================================
+ Hits         9028    9032       +4     
+ Misses       1422    1421       -1
Impacted Files Coverage Δ
.../op/stages/impl/feature/TextMapNullEstimator.scala 100% <ø> (ø) ⬆️
...la/com/salesforce/op/stages/OpPipelineStages.scala 62.5% <ø> (-1.02%) ⬇️
...sforce/op/stages/impl/feature/Transmogrifier.scala 96.9% <ø> (ø) ⬆️
...rce/op/stages/impl/preparators/SanityChecker.scala 91.59% <0%> (-0.25%) ⬇️
...m/salesforce/op/stages/OpPipelineStageParams.scala 91.66% <100%> (+0.49%) ⬆️
...orce/op/stages/impl/feature/BinaryVectorizer.scala 100% <100%> (ø) ⬆️
...e/op/stages/impl/feature/TextMapLenEstimator.scala 100% <100%> (ø) ⬆️
.../op/features/types/FeatureTypeSparkConverter.scala 99.11% <0%> (+0.88%) ⬆️
.../scala/com/salesforce/op/dsl/RichTextFeature.scala 86.56% <0%> (+1.49%) ⬆️

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 0e64ff6...e34c26f. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 11, 2019

Codecov Report

Merging #280 into master will decrease coverage by 26.25%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #280       +/-   ##
===========================================
- Coverage   86.39%   60.14%   -26.26%     
===========================================
  Files         319      319               
  Lines       10440    10692      +252     
  Branches      350      546      +196     
===========================================
- Hits         9020     6431     -2589     
- Misses       1420     4261     +2841
Impacted Files Coverage Δ
...la/com/salesforce/op/stages/OpPipelineStages.scala 36.98% <ø> (-26.53%) ⬇️
...m/salesforce/op/stages/OpPipelineStageParams.scala 91.66% <100%> (+0.49%) ⬆️
...ages/impl/feature/MultiPickListMapVectorizer.scala 0% <0%> (-100%) ⬇️
...la/com/salesforce/op/aggregators/Geolocation.scala 0% <0%> (-100%) ⬇️
.../salesforce/op/aggregators/FeatureAggregator.scala 0% <0%> (-100%) ⬇️
...lesforce/op/stages/impl/feature/LangDetector.scala 0% <0%> (-100%) ⬇️
...alesforce/op/cli/gen/templates/SimpleProject.scala 0% <0%> (-100%) ⬇️
...sforce/op/stages/base/unary/UnaryTransformer.scala 0% <0%> (-100%) ⬇️
...e/op/stages/impl/feature/TextMapLenEstimator.scala 0% <0%> (-100%) ⬇️
.../op/stages/base/sequence/SequenceTransformer.scala 0% <0%> (-100%) ⬇️
... and 145 more

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 6e95804...8d02a1c. Read the comment docs.

@tovbinm
Copy link
Collaborator

tovbinm commented Apr 11, 2019

@leahmcguire there are some build failures you might want to look into.

Copy link
Contributor

@gerashegalov gerashegalov left a comment

Choose a reason for hiding this comment

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

LGTM

@leahmcguire leahmcguire merged commit 3e9a815 into master Apr 12, 2019
@leahmcguire leahmcguire deleted the lm/onSetFix branch April 12, 2019 19:33
@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants