-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix build break #146
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
Fix build break #146
Conversation
The previous 2 changes conflicted. Resovling the break that happened between them.
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.
return trainerKind == TrainerKinds.SignatureBinaryClassifierTrainer; | ||
|
||
if (type == typeof(Trainers.LogisticRegressor)) | ||
if (type == typeof(Trainers.LogisticRegressionClassifier)) |
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.
LogisticRegressionClassifier [](start = 40, length = 28)
Thanks for the fix! #Resolved
@@ -197,10 +197,10 @@ public static T TrainerKindApiValue<T>(TrainerKinds trainerKind) | |||
|
|||
public static bool IsTrainerOfKind(Type type, TrainerKinds trainerKind) | |||
{ | |||
if (type == typeof(Trainers.BinaryLogisticRegressor)) | |||
if (type == typeof(Trainers.LogisticRegressionBinaryClassifier)) |
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.
LogisticRegressionBinaryClassifier [](start = 40, length = 34)
Thanks for fixing this Eric!
Actually, lines 200-204 are now not needed, they were there before only because the name for logistic classifiers didn't match the pattern of the other trainers (that the entry point names end with "Classifier" or "BinaryClassifier").
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.
Can you make a follow-up for that change? The goal here is to unblock the build. I don't want to add anymore risk.
The previous 2 changes conflicted. Resovling the break that happened between them.
The previous 2 changes conflicted. Resovling the break that happened between them.
I'll merge once green to get the build unblocked again.
/cc @TomFinley @glebuk @shauheen