-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Pass weighting value from ProduceWordBags to WordBagEstimator constru… #3135
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3135 +/- ##
==========================================
- Coverage 72.52% 72.52% -0.01%
==========================================
Files 808 808
Lines 144665 144665
Branches 16198 16198
==========================================
- Hits 104914 104912 -2
- Misses 35340 35343 +3
+ Partials 4411 4410 -1
|
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.
@@ -283,7 +283,7 @@ public static class TextCatalog | |||
int maximumNgramsCount = NgramExtractingEstimator.Defaults.MaximumNgramsCount, | |||
NgramExtractingEstimator.WeightingCriteria weighting = NgramExtractingEstimator.WeightingCriteria.Tf) | |||
=> new WordBagEstimator(Contracts.CheckRef(catalog, nameof(catalog)).GetEnvironment(), | |||
outputColumnName, inputColumnName, ngramLength, skipLength, useAllLengths, maximumNgramsCount); | |||
outputColumnName, inputColumnName, ngramLength, skipLength, useAllLengths, maximumNgramsCount, weighting); |
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.
weighting [](start = 111, length = 9)
Would be nice to add baseline test, something similar to TestWordEmbeddings
.
Thank you for catching and fixing it!
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! Thanks for the catching it.
Fixes #3134