Reformatting BinaryClassification samples to width 85 #3946

sierralee51 · 2019-07-02T02:25:39Z

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

sierralee51 · 2019-07-02T02:26:07Z

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

changing back

sierralee51 · 2019-07-02T04:23:43Z

I have to reformat my spacing again. I've noticed this happens primarily in .tt files and the files that use them.

Spacing should be good now.

sharwell

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.

rayankrish · 2019-07-02T18:55:01Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/AveragedPerceptron.cs

            Console.WriteLine($"F1 Score: {metrics.F1Score:F2}");
-            Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}");
+            Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}")
+                ;


I don't think this is the best formatting. Maybe do it like this:

Console.WriteLine($"Negative Precision: " + $"{metrics.NegativePrecision:F2}");

rayankrish · 2019-07-02T19:32:07Z

.../Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/AveragedPerceptronWithOptions.cs

-            var pipeline = mlContext.BinaryClassification.Trainers.AveragedPerceptron(options);
+            var pipeline = mlContext.BinaryClassification.Trainers
+                .AveragedPerceptron(options);
+


yeah noticed that a couple minutes ago...looking for the tt/ttinclude file that is probably causing it

rayankrish

Extra line after most pipeline set ups. lines with a single semicolon might need to be reformatted to be evenly distributed

rayankrish · 2019-07-02T19:49:09Z

...Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/FieldAwareFactorizationMachine.cs

                // Specify three feature columns!
-                new[] {nameof(DataPoint.Field0), nameof(DataPoint.Field1), nameof(DataPoint.Field2) },
+                new[] {nameof(DataPoint.Field0), nameof(DataPoint.Field1),
+            nameof(DataPoint.Field2) },


nameof should be on the same level of new[]. I think it needs 4 more spaces

rayankrish · 2019-07-02T19:50:12Z

...Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/FieldAwareFactorizationMachine.tt

                // Specify three feature columns!
-                new[] {nameof(DataPoint.Field0), nameof(DataPoint.Field1), nameof(DataPoint.Field2) },
+                new[] {nameof(DataPoint.Field0), nameof(DataPoint.Field1),
+            nameof(DataPoint.Field2) },


This is the root cause of the tab issue of nameof. It needs to be four spaces to the right.

rayankrish · 2019-07-02T20:44:24Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/AveragedPerceptron.cs

-            var pipeline = mlContext.BinaryClassification.Trainers.AveragedPerceptron();
+            var pipeline = mlContext.BinaryClassification.Trainers
+                .AveragedPerceptron();
+


rayankrish · 2019-07-02T20:45:28Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/FactorizationMachine.cs

-            var pipeline = mlContext.BinaryClassification.Trainers.FieldAwareFactorizationMachine();
+            var pipeline = mlContext.BinaryClassification.Trainers
+                .FieldAwareFactorizationMachine();
+


rayankrish · 2019-07-02T20:45:45Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/FastForest.cs

-            var pipeline = mlContext.BinaryClassification.Trainers.FastForest();
+            var pipeline = mlContext.BinaryClassification.Trainers
+                .FastForest();
+


rayankrish · 2019-07-02T20:48:26Z

...mples/Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/SgdCalibratedWithOptions.cs

-            var pipeline = mlContext.BinaryClassification.Trainers.SgdCalibrated(options);
+            var pipeline = mlContext.BinaryClassification.Trainers
+                .SgdCalibrated(options);
+


rayankrish · 2019-07-02T20:48:33Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/SgdNonCalibrated.cs

-            var pipeline = mlContext.BinaryClassification.Trainers.SgdNonCalibrated();
+            var pipeline = mlContext.BinaryClassification.Trainers
+            .SgdNonCalibrated();
+


rayankrish · 2019-07-02T20:48:39Z

...es/Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/SgdNonCalibratedWithOptions.cs

-            var pipeline = mlContext.BinaryClassification.Trainers.SgdNonCalibrated(options);
+            var pipeline = mlContext.BinaryClassification.Trainers
+            .SgdNonCalibrated(options);
+


rayankrish · 2019-07-02T20:48:46Z

.../Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/SymbolicSgdLogisticRegression.cs

-            var pipeline = mlContext.BinaryClassification.Trainers.SymbolicSgdLogisticRegression();
+            var pipeline = mlContext.BinaryClassification.Trainers
+                .SymbolicSgdLogisticRegression();
+


rayankrish · 2019-07-02T20:48:53Z

...ML.Samples/Dynamic/Trainers/BinaryClassification/SymbolicSgdLogisticRegressionWithOptions.cs

-            var pipeline = mlContext.BinaryClassification.Trainers.SymbolicSgdLogisticRegression(options);
+            var pipeline = mlContext.BinaryClassification.Trainers
+                .SymbolicSgdLogisticRegression(options);
+


sayanshaw24

Looks good.

sayanshaw24 · 2019-07-02T23:36:06Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/FastTree.cs

+                    Features = Enumerable.Repeat(label, 50)
+                        .Select(x => x ? randomFloat() : randomFloat() +
+                        0.03f).ToArray()
+


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.

sayanshaw24 · 2019-07-02T23:38:06Z

...Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/FieldAwareFactorizationMachine.cs

            Console.WriteLine($"F1 Score: {metrics.F1Score:F2}");
-            Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}");
+            Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}")
+                ;


Why are there semicolons by themselves? How about breaking after "Console.WriteLine(" ?

sayanshaw24 · 2019-07-02T23:38:21Z

...Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/FieldAwareFactorizationMachine.cs

-            Console.WriteLine($"Positive Precision: {metrics.PositivePrecision:F2}");
+            Console.WriteLine($"Positive Precision: {metrics.PositivePrecision:F2}")
+                ;
+


Look at previous comment

sayanshaw24 · 2019-07-02T23:40:19Z

...L.Samples/Dynamic/Trainers/BinaryClassification/FieldAwareFactorizationMachineWithOptions.tt

-                ExtraFeatureColumns = new[] { nameof(DataPoint.Field1), nameof(DataPoint.Field2) },
+                ExtraFeatureColumns = 
+            new[] { nameof(DataPoint.Field1), nameof(DataPoint.Field2) },
+


Is the indentation in line 24 of this tt file different than the norms we set because of spacing issues in the cs file?

sayanshaw24 · 2019-07-02T23:41:23Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/GamWithOptions.cs

-                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;



I think the + should be on line 161.

rayankrish · 2019-07-02T23:45:25Z

...Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/FieldAwareFactorizationMachine.cs

            Console.WriteLine($"F1 Score: {metrics.F1Score:F2}");
-            Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}");
+            Console.WriteLine($"Negative Precision: {metrics.NegativePrecision:F2}")
+                ;


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

reformatted BinaryClassification samples

cdf6b36

sierralee51 added 28 commits July 1, 2019 19:29

Update AveragedPerceptron.cs

7abb9d1

fixing spacing

Update AveragedPerceptronWithOptions.cs

5c3214b

fixing whitespace

Update AveragedPerceptron.cs

74935ee

Update AveragedPerceptron.cs

e9891f9

Update BinaryClassification.ttinclude

c9dcd03

fixing whitespace

Update FactorizationMachine.cs

384f983

fixing whitespace

Update FastForest.cs

885f820

fixing whitespace

Update FastForestWithOptions.cs

1d1679b

fixing whitespace

Update FastTree.cs

f3ec594

fixing whitespace

Update FastTreeWithOptions.cs

fb62ed5

fixing whitespace

Update FieldAwareFactorizationMachine.cs

c243e2e

fixing whitespace

Update FieldAwareFactorizationMachine.cs

5458e79

Update FieldAwareFactorizationMachine.tt

60130f3

fixing whitespace

Update FieldAwareFactorizationMachineWithOptions.cs

edcb4f5

fixing whitespace

Update FieldAwareFactorizationMachine.cs

3d8613e

Update FieldAwareFactorizationMachineWithOptions.tt

5aabe0e

fixing whitespace

Update LbfgsLogisticRegression.cs

85036e2

fixing whitespace

Update LbfgsLogisticRegressionWithOptions.cs

92c6c19

fixing whitespace

Update LightGbm.cs

f0990a1

fixing whitespace

Update LightGbmWithOptions.cs

672b785

fixing whitespace

Update LinearSvm.cs

c41905d

fixing whitespace

Update LinearSvmWithOptions.cs

e93c699

fixing whitespace

Update MultipleFeatureColumnsBinaryClassification.ttinclude

cbbee59

fixing whitespace

Update PriorTrainer.cs

2fc5e20

fixing whitespace

Update AveragedPerceptron.cs

a43123c

Update AveragedPerceptronWithOptions.cs

58484e2

Update BinaryClassification.ttinclude

a42c2e8

Update FactorizationMachine.cs

a9afb72

sierralee51 added 13 commits July 1, 2019 20:51

Update SdcaLogisticRegression.cs

1b05112

Update SgdCalibrated.cs

47481ed

Update SgdCalibratedWithOptions.cs

371e043

Update SgdNonCalibrated.cs

254fdef

Update SgdNonCalibratedWithOptions.cs

b860f30

Update SymbolicSgdLogisticRegression.cs

664aa71

Update SymbolicSgdLogisticRegressionWithOptions.cs

f6871c8

Update Program.cs

539fe74

changing back

Update Program.cs

ed51348

Update Program.cs

d112945

Update Program.cs

3af17a8

Update Program.cs

93a98fe

Update Program.cs

652f39e

sierralee51 requested review from codemzs, rayankrish and sayanshaw24 July 2, 2019 05:04

sharwell suggested changes Jul 2, 2019

View reviewed changes

fixed tab issues

0857a13

rayankrish reviewed Jul 2, 2019

View reviewed changes

rayankrish approved these changes Jul 2, 2019

View reviewed changes

fixed indentations

6eac51a

sayanshaw24 approved these changes Jul 2, 2019

View reviewed changes

rayankrish reviewed Jul 2, 2019

View reviewed changes

fixed commented-on parts

109a4cd

sierralee51 merged commit ce9b38b into dotnet:master Jul 3, 2019

ghost locked as resolved and limited conversation to collaborators Mar 21, 2022

Reformatting BinaryClassification samples to width 85 #3946

Reformatting BinaryClassification samples to width 85 #3946

Uh oh!

Conversation

sierralee51 commented Jul 2, 2019

Uh oh!

sierralee51 commented Jul 2, 2019

Uh oh!

sierralee51 commented Jul 2, 2019

Uh oh!

sharwell left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rayankrish Jul 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rayankrish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sayanshaw24 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sharwell left a comment •

edited

Loading

rayankrish Jul 2, 2019 •

edited

Loading