Reformatting MulticlassClassification samples to width 85 #3942

sierralee51 · 2019-07-01T20:41:03Z

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
*There are a couple exceptions to the 85 char. limit that have been approved by Natalie

sierralee51 · 2019-07-01T20:42:30Z

Just noticed that, like last time, there are some extra spaces before comments that don't show up on VisualStudio. Fixing them through GitHub right now.

getting rid of whitespace

fixing whitespace

sierralee51 · 2019-07-01T21:56:00Z

Just noticed that, like last time, there are some extra spaces before comments that don't show up on VisualStudio. Fixing them through GitHub right now.

In fixing these extra spaces before comments, I also realized that all my indentations were messed up. The extra spaces in my PR should be fixed, and I am now investigating the issue of extra whitespace in GitHub, as I know that Sayan and I both encountered this despite using spaces rather than tabs. A quick search has shown that this is a relatively common isssue, so I'll be looking into ways to prevent this in the future.

…to multiclassclassification2

codemzs · 2019-07-02T05:42:30Z

...amples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LbfgsMaximumEntropy.cs

-            // as a catalog of available operations and as the source of randomness.
-            // Setting the seed to a fixed number in this example to make outputs deterministic.
+            // Create a new context for ML.NET operations. It can be used for
+	    // exception tracking and logging, as a catalog of available operations


[](start = 0, length = 2)

why do you have these tabs?

codemzs · 2019-07-02T05:44:12Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LightGbm.cs

-            var predictions = mlContext.Data.CreateEnumerable<Prediction>(transformedTestData, reuseRowObject: false).ToList();
+            var predictions = mlContext.Data
+                .CreateEnumerable<Prediction>(transformedTestData,
+		reuseRowObject: false).ToList();


[](start = 0, length = 2)

tabs. please convert tabs to spaces.
https://blogs.msdn.microsoft.com/zainnab/2010/03/14/how-to-convert-tabs-to-spaces-and-vice-versa/

codemzs · 2019-07-02T05:44:50Z

...soft.ML.Samples/Dynamic/Trainers/MulticlassClassification/MulticlassClassification.ttinclude

                };
            }
        }

-        // Example with label and 20 feature values. A data set is a collection of such examples.
+        // Example with label and 20 feature values. A data set is a collection of
+	// such examples.


Align the //

codemzs · 2019-07-02T05:46:01Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/NaiveBayes.cs

-            var predictions = mlContext.Data.CreateEnumerable<Prediction>(transformedTestData, reuseRowObject: false).ToList();
+            var predictions = mlContext.Data
+                .CreateEnumerable<Prediction>(transformedTestData,
+		reuseRowObject: false).ToList();


reuseRowObject: false).ToList(); [](start = 2, length = 32)

this needs to be indented after .CreateEnumerable with 4 splaces.

codemzs · 2019-07-02T05:46:19Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/NaiveBayes.cs


            // Look at 5 predictions
            foreach (var p in predictions.Take(5))
-                Console.WriteLine($"Label: {p.Label}, Prediction: {p.PredictedLabel}");
+                Console.WriteLine($"Label: {p.Label}, " + 
+		    $"Prediction: {p.PredictedLabel}");


$"Prediction: {p.PredictedLabel}"); [](start = 6, length = 35)

same here but after Console.

codemzs · 2019-07-02T05:46:32Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/NaiveBayes.cs

@@ -56,7 +69,9 @@ public static void Example()
            //   Label: 3, Prediction: 3

            // Evaluate the overall metrics
-            var metrics = mlContext.MulticlassClassification.Evaluate(transformedTestData);
+            var metrics = mlContext.MulticlassClassification
+	        .Evaluate(transformedTestData);


.Evaluate(transformedTestData); [](start = 9, length = 31)

indent

codemzs · 2019-07-02T05:46:47Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/NaiveBayes.tt

+        // though they may be dependent on each other. It is  a multi-class trainer
+	// that accepts binary feature values of type float, i.e., feature values
+	// are either true or false. Specifically a feature value greater than zero
+	// is treated as true, zero or less is treated as false.";


Alight all //

codemzs · 2019-07-02T05:47:56Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/PairwiseCoupling.cs

+                    // Apply PairwiseCoupling multiclass meta trainer on top of
+		    // binary trainer.
+                    .Append(mlContext.MulticlassClassification.Trainers
+		        .PairwiseCoupling(


.PairwiseCoupling( [](start = 10, length = 18)

indent after .Append with 4 spaces

codemzs · 2019-07-02T05:48:28Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/PairwiseCoupling.cs

+		    // binary trainer.
+                    .Append(mlContext.MulticlassClassification.Trainers
+		        .PairwiseCoupling(
+			mlContext.BinaryClassification.Trainers.SdcaLogisticRegression()));


mlContext.BinaryClassification.Trainers.SdcaLogisticRegression())); [](start = 3, length = 67)

indent after .PairwiseCoupling with 4 spaces.

codemzs · 2019-07-02T05:48:50Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/PairwiseCoupling.cs

+            // Create testing data. Use different random seed to make it different
+	    // from training data.
+            var testData = mlContext.Data
+	        .LoadFromEnumerable(GenerateRandomDataPoints(500, seed: 123));


.LoadFromEnumerable(GenerateRandomDataPoints(500, seed: 123)); [](start = 9, length = 62)

indent after var with 4 spaces.

codemzs · 2019-07-02T05:49:38Z

It seems the indentation needs to be fixed. My comments apply at many places where I have not commented.

sharwell · 2019-07-02T14:39:39Z

...amples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LbfgsMaximumEntropy.cs

-                    mlContext.Transforms.Conversion.MapValueToKey(nameof(DataPoint.Label))
-                    // Apply LbfgsMaximumEntropy multiclass trainer.
-                    .Append(mlContext.MulticlassClassification.Trainers.LbfgsMaximumEntropy());
+            // Convert the string labels into key types.


📝 Line should be indented by 4 more spaces

sharwell · 2019-07-02T14:40:05Z

...amples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LbfgsMaximumEntropy.cs

-                    .Append(mlContext.MulticlassClassification.Trainers.LbfgsMaximumEntropy());
+            // Convert the string labels into key types.
+                mlContext.Transforms.Conversion
+                .MapValueToKey(nameof(DataPoint.Label))


📝 Line should be indented by 4 more spaces

sharwell · 2019-07-02T14:40:54Z

...amples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LbfgsMaximumEntropy.cs

+                .MapValueToKey(nameof(DataPoint.Label))
+            // Apply LbfgsMaximumEntropy multiclass trainer.
+                .Append(mlContext.MulticlassClassification.Trainers
+                .LbfgsMaximumEntropy());


Wrap the entire argument if possible (before mlContext)

Indent this line an additional 4 spaces (8 total from current location)

sharwell · 2019-07-02T14:42:02Z

...amples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LbfgsMaximumEntropy.cs

-            var predictions = mlContext.Data.CreateEnumerable<Prediction>(transformedTestData, reuseRowObject: false).ToList();
+            var predictions = mlContext.Data
+                .CreateEnumerable<Prediction>(transformedTestData,
+                reuseRowObject: false).ToList();


📝 This is difficult to read. See preferred form in #3941 (comment)

sharwell · 2019-07-02T14:42:31Z

...amples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LbfgsMaximumEntropy.cs


            // Look at 5 predictions
            foreach (var p in predictions.Take(5))
-                Console.WriteLine($"Label: {p.Label}, Prediction: {p.PredictedLabel}");
+                Console.WriteLine($"Label: {p.Label}, " + 
+                    $"Prediction: {p.PredictedLabel}");


📝 Multi-line body of foreach needs braces

sharwell · 2019-07-02T14:43:04Z

...amples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LbfgsMaximumEntropy.cs

+        // Generates random uniform doubles in [-0.5, 0.5)
+        // range with labels 1, 2 or 3.
+        private static IEnumerable<DataPoint> GenerateRandomDataPoints(int count,
+            int seed=0)


📝 Wrap no arguments, or wrap all arguments with one argument per line

sharwell · 2019-07-02T14:43:46Z

...amples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LbfgsMaximumEntropy.tt

@@ -8,7 +8,8 @@ string TrainerOptions = null;
 string OptionsInclude = "";
 string Comments = "";
 bool CacheData = false;
-string DataGenerationComments= "// Generates random uniform doubles in [-0.5, 0.5) range with labels 1, 2 or 3.";
+string DataGenerationComments= "// Generates random uniform doubles in [-0.5, 0.5)"
+    + "\n        // range with labels 1, 2 or 3.";


📝 Place \n at the end of the preceding line

sharwell · 2019-07-02T14:44:26Z

...osoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LbfgsMaximumEntropyWithOptions.cs

-                    // Apply LbfgsMaximumEntropy multiclass trainer.
-                    .Append(mlContext.MulticlassClassification.Trainers.LbfgsMaximumEntropy(options));
-
+            // Convert the string labels into key types.


📝 Needs 4 more spaces

sharwell · 2019-07-02T14:44:49Z

...osoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LbfgsMaximumEntropyWithOptions.cs

+                mlContext.Transforms.Conversion.MapValueToKey("Label")
+            // Apply LbfgsMaximumEntropy multiclass trainer.
+                .Append(mlContext.MulticlassClassification.Trainers
+                .LbfgsMaximumEntropy(options));


📝 See earlier comment regarding this wrapping

...amples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LightGbmWithOptions.cs

rayankrish · 2019-07-02T21:18:52Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/OneVersusAll.cs

+                .Append(mlContext.MulticlassClassification.Trainers
+                .OneVersusAll(
+                mlContext.BinaryClassification.Trainers.SdcaLogisticRegression()));
+


rayankrish · 2019-07-02T21:19:02Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/PairwiseCoupling.cs

+                .Append(mlContext.MulticlassClassification.Trainers
+                .PairwiseCoupling(
+                mlContext.BinaryClassification.Trainers.SdcaLogisticRegression()));
+


sayanshaw24

Looks good to me. There are just a few indents you could add, and empty lines you could remove (@RadicalRayan noted most of them). Take a look at our comments and merge when the tests pass.

sayanshaw24 · 2019-07-02T22:51:35Z

...osoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/LbfgsMaximumEntropyWithOptions.tt

@@ -12,7 +12,9 @@ string TrainerOptions = @"LbfgsMaximumEntropyMulticlassTrainer.Options

 string OptionsInclude = "using Microsoft.ML.Trainers;";
 string Comments = "";
-string DataGenerationComments= "// Generates random uniform doubles in [-0.5, 0.5) range with labels 1, 2 or 3.";
+string DataGenerationComments= "// Generates random uniform doubles in [-0.5, 0.5)"
+    + "\n        // range with labels 1, 2 or 3.";


Maybe indent the comment "// range with labels . . ." with the same indent as the comment above it "Generates random . . . " ? There are more instances of this, so I guess there's a tt file you'd have to change.

That isn't a comment. It's part of the string

sayanshaw24 · 2019-07-02T22:55:36Z

...rosoft.ML.Samples/Dynamic/Trainers/MulticlassClassification/SdcaMaximumEntropyWithOptions.cs

+                // Apply SdcaMaximumEntropy multiclass trainer.
+                .Append(mlContext.MulticlassClassification.Trainers
+                .SdcaMaximumEntropy(options));
+


* reformatted multiclassclassification samples * fixing errors * reformatted MulticlassClassification samples * Update LbfgsMaximumEntropy.cs getting rid of whitespace * Update LbfgsMaximumEntropy.cs * Update LbfgsMaximumEntropyWithOptions.cs getting rid of whitespace * Update LightGbmWithOptions.cs fixing whitespace * Update LbfgsMaximumEntropy.cs * Update LightGbm.cs fixing whitespace * Update LightGbm.cs fixing whitespace * Update LightGbmWithOptions.cs fixing whitespace * Update MulticlassClassification.ttinclude fixing whitespace * Update MulticlassClassification.ttinclude fixing whitespace * Update NaiveBayes.cs fixing whitespace * Update NaiveBayes.tt fixing whitespace * Update NaiveBayes.tt * Update OneVersusAll.cs fixing whitespace * Update PairwiseCoupling.cs fixing whitespace * Update SdcaMaximumEntropy.cs fixing whitespace * Update SdcaMaximumEntropyWithOptions.cs fixing whitespace * Update SdcaNonCalibrated.cs fixing whitespace * Update SdcaNonCalibratedWithOptions.cs fixing whitespace * Update SdcaNonCalibrated.cs fixing whitespace * Update SdcaNonCalibrated.cs * Update LbfgsMaximumEntropy.cs * Update LbfgsMaximumEntropy.cs * Update LbfgsMaximumEntropyWithOptions.cs * Update LightGbm.cs * Update LightGbmWithOptions.cs * Update MulticlassClassification.ttinclude * Update NaiveBayes.cs * Update OneVersusAll.cs * Update PairwiseCoupling.cs * Update SdcaMaximumEntropy.cs * Update SdcaMaximumEntropy.cs * Update SdcaMaximumEntropyWithOptions.cs * Update SdcaNonCalibrated.cs * Update SdcaNonCalibratedWithOptions.cs * fixed tabbing issue * fixed indentations * aligned comments * fixed some indentation and spacing issues * fixed extra empty lines * fixed some more indentation issue

sierralee51 added 3 commits July 1, 2019 13:12

reformatted multiclassclassification samples

539a0f7

fixing errors

d18c419

reformatted MulticlassClassification samples

0b4aff6

sierralee51 requested review from codemzs, natke, wschin and artidoro July 1, 2019 20:41

sierralee51 added 22 commits July 1, 2019 13:43

Update LbfgsMaximumEntropy.cs

f84939f

getting rid of whitespace

Update LbfgsMaximumEntropy.cs

130c6da

Update LbfgsMaximumEntropyWithOptions.cs

71e4c19

getting rid of whitespace

Update LightGbmWithOptions.cs

8166627

fixing whitespace

Update LbfgsMaximumEntropy.cs

5ea92f3

Update LightGbm.cs

b63c891

fixing whitespace

Update LightGbm.cs

4d37328

fixing whitespace

Update LightGbmWithOptions.cs

6421a8b

fixing whitespace

Update MulticlassClassification.ttinclude

79093c8

fixing whitespace

Update MulticlassClassification.ttinclude

4c2f723

fixing whitespace

Update NaiveBayes.cs

4a48dc5

fixing whitespace

Update NaiveBayes.tt

cc1b3f8

fixing whitespace

Update NaiveBayes.tt

c6d4cfd

Update OneVersusAll.cs

97b3c99

fixing whitespace

Update PairwiseCoupling.cs

6be7d8a

fixing whitespace

Update SdcaMaximumEntropy.cs

3f74187

fixing whitespace

Update SdcaMaximumEntropyWithOptions.cs

0d23ad8

fixing whitespace

Update SdcaNonCalibrated.cs

c728577

fixing whitespace

Update SdcaNonCalibratedWithOptions.cs

6e82a10

fixing whitespace

Update SdcaNonCalibrated.cs

1f1ff3b

fixing whitespace

Update SdcaNonCalibrated.cs

239e26a

Update LbfgsMaximumEntropy.cs

32db971

sierralee51 added 3 commits July 1, 2019 14:41

Update SdcaMaximumEntropyWithOptions.cs

053c85b

Update SdcaNonCalibrated.cs

0d2baa6

Update SdcaNonCalibratedWithOptions.cs

6e410cc

Merge branch 'master' of https://github.com/dotnet/machinelearning in…

278b743

…to multiclassclassification2

codemzs reviewed Jul 2, 2019

View reviewed changes

sierralee51 added 3 commits July 1, 2019 23:24

fixed tabbing issue

85ed0e6

fixed indentations

f51c147

aligned comments

1e9a3d8

sierralee51 requested review from sayanshaw24 and rayankrish July 2, 2019 07:02

sharwell requested changes Jul 2, 2019

View reviewed changes

fixed some indentation and spacing issues

c3d3499

rayankrish approved these changes Jul 2, 2019

View reviewed changes

fixed extra empty lines

e497794

sayanshaw24 approved these changes Jul 2, 2019

View reviewed changes

fixed some more indentation issue

6224ed5

sierralee51 merged commit 4fecfb2 into dotnet:master Jul 3, 2019

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reformatting MulticlassClassification samples to width 85 #3942

Reformatting MulticlassClassification samples to width 85 #3942

sierralee51 commented Jul 1, 2019

sierralee51 commented Jul 1, 2019

sierralee51 commented Jul 1, 2019

codemzs Jul 2, 2019

codemzs Jul 2, 2019

codemzs Jul 2, 2019

codemzs Jul 2, 2019

codemzs Jul 2, 2019

codemzs Jul 2, 2019

codemzs Jul 2, 2019

codemzs Jul 2, 2019

codemzs Jul 2, 2019

codemzs Jul 2, 2019

codemzs commented Jul 2, 2019

sharwell Jul 2, 2019

sharwell Jul 2, 2019

sharwell Jul 2, 2019

sharwell Jul 2, 2019

sharwell Jul 2, 2019

sharwell Jul 2, 2019

sharwell Jul 2, 2019

sharwell Jul 2, 2019

sharwell Jul 2, 2019

rayankrish Jul 2, 2019

rayankrish Jul 2, 2019

sayanshaw24 left a comment

sayanshaw24 Jul 2, 2019

sierralee51 Jul 2, 2019

sayanshaw24 Jul 2, 2019

Reformatting MulticlassClassification samples to width 85 #3942

Reformatting MulticlassClassification samples to width 85 #3942

Conversation

sierralee51 commented Jul 1, 2019

sierralee51 commented Jul 1, 2019

sierralee51 commented Jul 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codemzs commented Jul 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sayanshaw24 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment