-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Reformatting BinaryClassification samples to width 85 #3946
Conversation
I have to reformat my spacing again. I've noticed this happens primarily in .tt files and the files that use them. |
fixing spacing
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
fixing whitespace
changing back
Spacing should be good now. |
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.
Requesting changes. The readability of these reformat pull requests is especially bad. I left a bunch of notes in one of the earlier ones, but those notes do not cover all situations where this is worse than before.
@@ -111,11 +129,16 @@ private static void PrintMetrics(BinaryClassificationMetrics metrics) | |||
Console.WriteLine($"Accuracy: {metrics.Accuracy:F2}"); | |||
Console.WriteLine($"AUC: {metrics.AreaUnderRocCurve:F2}"); | |||
Console.WriteLine($"F1 Score: {metrics.F1Score:F2}"); | |||
Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}"); | |||
Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}") | |||
; |
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.
I don't think this is the best formatting. Maybe do it like this:
Console.WriteLine($"Negative Precision: " +
$"{metrics.NegativePrecision:F2}");
var pipeline = mlContext.BinaryClassification.Trainers.AveragedPerceptron(options); | ||
var pipeline = mlContext.BinaryClassification.Trainers | ||
.AveragedPerceptron(options); | ||
|
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.
extra line
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.
yeah noticed that a couple minutes ago...looking for the tt/ttinclude file that is probably causing 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.
Extra line after most pipeline set ups. lines with a single semicolon might need to be reformatted to be evenly distributed
// Specify three feature columns! | ||
new[] {nameof(DataPoint.Field0), nameof(DataPoint.Field1), nameof(DataPoint.Field2) }, | ||
new[] {nameof(DataPoint.Field0), nameof(DataPoint.Field1), | ||
nameof(DataPoint.Field2) }, |
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.
nameof should be on the same level of new[]. I think it needs 4 more spaces
@@ -3,19 +3,25 @@ | |||
string ClassName="FieldAwareFactorizationMachine"; | |||
string Trainer = @"FieldAwareFactorizationMachine( | |||
// Specify three feature columns! | |||
new[] {nameof(DataPoint.Field0), nameof(DataPoint.Field1), nameof(DataPoint.Field2) }, | |||
new[] {nameof(DataPoint.Field0), nameof(DataPoint.Field1), | |||
nameof(DataPoint.Field2) }, |
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.
This is the root cause of the tab issue of nameof. It needs to be four spaces to the right.
var pipeline = mlContext.BinaryClassification.Trainers.AveragedPerceptron(); | ||
var pipeline = mlContext.BinaryClassification.Trainers | ||
.AveragedPerceptron(); | ||
|
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.
extra line
var pipeline = mlContext.BinaryClassification.Trainers.FieldAwareFactorizationMachine(); | ||
var pipeline = mlContext.BinaryClassification.Trainers | ||
.FieldAwareFactorizationMachine(); | ||
|
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.
extra line
var pipeline = mlContext.BinaryClassification.Trainers.FastForest(); | ||
var pipeline = mlContext.BinaryClassification.Trainers | ||
.FastForest(); | ||
|
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.
extra line
var pipeline = mlContext.BinaryClassification.Trainers.SgdCalibrated(options); | ||
var pipeline = mlContext.BinaryClassification.Trainers | ||
.SgdCalibrated(options); | ||
|
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.
extra line
var pipeline = mlContext.BinaryClassification.Trainers.SgdNonCalibrated(); | ||
var pipeline = mlContext.BinaryClassification.Trainers | ||
.SgdNonCalibrated(); | ||
|
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.
extra line
var pipeline = mlContext.BinaryClassification.Trainers.SgdNonCalibrated(options); | ||
var pipeline = mlContext.BinaryClassification.Trainers | ||
.SgdNonCalibrated(options); | ||
|
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.
extra line
var pipeline = mlContext.BinaryClassification.Trainers.SymbolicSgdLogisticRegression(); | ||
var pipeline = mlContext.BinaryClassification.Trainers | ||
.SymbolicSgdLogisticRegression(); | ||
|
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.
extra line
var pipeline = mlContext.BinaryClassification.Trainers.SymbolicSgdLogisticRegression(options); | ||
var pipeline = mlContext.BinaryClassification.Trainers | ||
.SymbolicSgdLogisticRegression(options); | ||
|
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.
extra line
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.
Looks good.
Features = Enumerable.Repeat(label, 50) | ||
.Select(x => x ? randomFloat() : randomFloat() + | ||
0.03f).ToArray() | ||
|
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.
Will that entire ternary operator fit on one line? I think (x => x ? randomFloat() : randomFloat() + 0.03f) should be on one line, if it fits.
{ | ||
Console.WriteLine($"Accuracy: {metrics.Accuracy:F2}"); | ||
Console.WriteLine($"AUC: {metrics.AreaUnderRocCurve:F2}"); | ||
Console.WriteLine($"F1 Score: {metrics.F1Score:F2}"); | ||
Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}"); | ||
Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}") | ||
; |
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.
Why are there semicolons by themselves? How about breaking after "Console.WriteLine(" ?
Console.WriteLine($"Positive Precision: {metrics.PositivePrecision:F2}"); | ||
Console.WriteLine($"Positive Precision: {metrics.PositivePrecision:F2}") | ||
; | ||
|
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.
Look at previous comment
ExtraFeatureColumns = new[] { nameof(DataPoint.Field1), nameof(DataPoint.Field2) }, | ||
ExtraFeatureColumns = | ||
new[] { nameof(DataPoint.Field1), nameof(DataPoint.Field2) }, | ||
|
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.
Is the indentation in line 24 of this tt file different than the norms we set because of spacing issues in the cs file?
@@ -140,7 +158,8 @@ private static IEnumerable<Data> GenerateData(int numExamples = 25000, int seed | |||
Features = new float[2] { centeredFloat(), centeredFloat() } | |||
}; | |||
// Compute the label from the shape functions and add noise. | |||
data.Label = Sigmoid(Parabola(data.Features[0]) + SimplePiecewise(data.Features[1]) + centeredFloat()) > 0.5; | |||
data.Label = Sigmoid(Parabola(data.Features[0]) | |||
+ SimplePiecewise(data.Features[1]) + centeredFloat()) > 0.5; | |||
|
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.
I think the + should be on line 161.
{ | ||
Console.WriteLine($"Accuracy: {metrics.Accuracy:F2}"); | ||
Console.WriteLine($"AUC: {metrics.AreaUnderRocCurve:F2}"); | ||
Console.WriteLine($"F1 Score: {metrics.F1Score:F2}"); | ||
Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}"); | ||
Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}") | ||
; |
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.
semicolon on its own line
* reformatted BinaryClassification samples * Update AveragedPerceptron.cs fixing spacing * Update AveragedPerceptronWithOptions.cs fixing whitespace * Update AveragedPerceptron.cs * Update AveragedPerceptron.cs * Update BinaryClassification.ttinclude fixing whitespace * Update FactorizationMachine.cs fixing whitespace * Update FastForest.cs fixing whitespace * Update FastForestWithOptions.cs fixing whitespace * Update FastTree.cs fixing whitespace * Update FastTreeWithOptions.cs fixing whitespace * Update FieldAwareFactorizationMachine.cs fixing whitespace * Update FieldAwareFactorizationMachine.cs * Update FieldAwareFactorizationMachine.tt fixing whitespace * Update FieldAwareFactorizationMachineWithOptions.cs fixing whitespace * Update FieldAwareFactorizationMachine.cs * Update FieldAwareFactorizationMachineWithOptions.tt fixing whitespace * Update LbfgsLogisticRegression.cs fixing whitespace * Update LbfgsLogisticRegressionWithOptions.cs fixing whitespace * Update LightGbm.cs fixing whitespace * Update LightGbmWithOptions.cs fixing whitespace * Update LinearSvm.cs fixing whitespace * Update LinearSvmWithOptions.cs fixing whitespace * Update MultipleFeatureColumnsBinaryClassification.ttinclude fixing whitespace * Update PriorTrainer.cs fixing whitespace * Update AveragedPerceptron.cs * Update AveragedPerceptronWithOptions.cs * Update BinaryClassification.ttinclude * Update FactorizationMachine.cs * Update FastForestWithOptions.cs * Update FastTree.cs * Update FastTreeWithOptions.cs * Update LbfgsLogisticRegression.cs * Update LbfgsLogisticRegressionWithOptions.cs * Update LightGbm.cs * Update LightGbmWithOptions.cs * Update LinearSvm.cs * Update LinearSvmWithOptions.cs * Update SdcaLogisticRegression.cs * Update SdcaLogisticRegressionWithOptions.cs * Update SdcaNonCalibrated.cs * Update SdcaNonCalibratedWithOptions.cs * Update SdcaNonCalibrated.cs * Update SdcaLogisticRegressionWithOptions.cs * Update SdcaLogisticRegression.cs * Update SgdCalibrated.cs * Update SgdCalibratedWithOptions.cs * Update SgdNonCalibrated.cs * Update SgdNonCalibratedWithOptions.cs * Update SymbolicSgdLogisticRegression.cs * Update SymbolicSgdLogisticRegressionWithOptions.cs * Update Program.cs changing back * Update Program.cs * Update Program.cs * Update Program.cs * Update Program.cs * Update Program.cs * fixed tab issues * fixed indentations * fixed commented-on parts
Guidelines followed:
-85 characters per line
-Use 4 spaces for indentation
-Dot and open parentheses stay on same line as function
-If not a preexisting line under line that we break, add an extra line after it
-Don't indent comments
-Don't break a comment if it represents output
-Don't break links
-If applicable, break right before $
-Keep math op together
Fix for issue #3478