Reformatted Recommendation samples to width 85 #3941

sayanshaw24 · 2019-07-01T19:31:25Z

Guidelines followed:

85 characters per line
Use 4 spaces for indentation
Dot, open parentheses, and function/variable name on same line
Math operations on same line
Don't indent comments
Don't break links
Don't break a comment if it represents a print output
Add an extra line after a break if it does not already exist
Break before "$"
Fix for Issue Samples width: they need to be formatted at 85 characters or less. #3478

wschin · 2019-07-01T19:57:58Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Recommendation/MatrixFactorization.cs

-                    dataMatrix.Add(new MatrixElement() { MatrixColumnIndex = i, MatrixRowIndex = j, Value = (i + j) % 5 });
+                    dataMatrix.Add(new MatrixElement() { MatrixColumnIndex = i,
+                        MatrixRowIndex = j, Value = (i + j) % 5 });
+


Empty line. Is it produced by an auto-formatting tool?

Actually, Zeeshan asked us to do that, so that's what we've been doing as a norm.

wschin · 2019-07-01T19:59:34Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Recommendation/MatrixFactorization.cs

-            Console.WriteLine($"Root Mean Squared Error: {metrics.RootMeanSquaredError:F2}");
-            Console.WriteLine($"RSquared: {metrics.RSquared:F2}");
+            Console.WriteLine("Mean Absolute Error: " + metrics.MeanAbsoluteError +
+                ":F2");


Is this correct? I thought F2 tells the prcision.

I'm not sure either. I tried reformatting it like another example Zeeshan helped us with that used a similar format. Do you know what the right notation is?

wschin · 2019-07-01T20:01:00Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Recommendation/MatrixFactorization.cs

+                ":F2");
+
+            Console.WriteLine("Mean Squared Error: " + metrics.MeanSquaredError +
+                ":F2");


Maybe merging the two lines to

Console.WriteLine("Mean Squared Error: " + metrics.MeanSquaredError);

could be a reasonable setting.

...mples/Microsoft.ML.Samples/Dynamic/Trainers/Recommendation/MatrixFactorizationWithOptions.cs

...mples/Microsoft.ML.Samples/Dynamic/Trainers/Recommendation/MatrixFactorizationWithOptions.tt

wschin · 2019-07-01T20:06:55Z

...crosoft.ML.Samples/Dynamic/Trainers/Recommendation/OneClassMatrixFactorizationWithOptions.cs

-                // its desired value would be set 0.15. In other words, this parameter determines
-                // the value of all missing matrix elements.
+                MatrixColumnIndexColumnName = nameof(
+                    MatrixElement.MatrixColumnIndex),


Line 46 and Lines 47-53 should be at the same indent level.

Zeeshan asked us to indent after the first break, so we've been doing that for all the files. Maybe we can discuss this along with Zeeshan in the scrum tomorrow?

…Type error

codemzs

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

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Recommendation/MatrixFactorization.cs

-            var predictions = mlContext.Data.CreateEnumerable<MatrixElement>(transformedData, reuseRowObject: false).Take(5).ToList();
+            var predictions = mlContext.Data
+                .CreateEnumerable<MatrixElement>(transformedData,
+                reuseRowObject: false).Take(5).ToList();


📝 This layout is very difficult to read. The following would be preferable:

var predictions = mlContext.Data .CreateEnumerable<MatrixElement>( transformedData, reuseRowObject: false) .Take(5) .ToList();

sharwell · 2019-07-02T14:29:09Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Recommendation/MatrixFactorization.cs

-                   nameof(MatrixElement.MatrixRowIndex), 10, 0.2, 1);
+            var pipeline = mlContext.Recommendation().Trainers.
+                MatrixFactorization(nameof(MatrixElement.Value),
+                nameof(MatrixElement.MatrixColumnIndex),


📝 This is incorrectly indented

sharwell · 2019-07-02T14:29:36Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Recommendation/MatrixFactorization.cs

@@ -74,32 +92,40 @@ private static List<MatrixElement> GenerateMatrix()
            var dataMatrix = new List<MatrixElement>();
            for (uint i = 0; i < MatrixColumnCount; ++i)
                for (uint j = 0; j < MatrixRowCount; ++j)
-                    dataMatrix.Add(new MatrixElement() { MatrixColumnIndex = i, MatrixRowIndex = j, Value = (i + j) % 5 });
+                    dataMatrix.Add(new MatrixElement() { MatrixColumnIndex = i,
+                        MatrixRowIndex = j, Value = (i + j) % 5 });


📝 Incorrect line wrapping and indentation

sharwell

Requesting changes. The wrapping applied in this pull request produces worse readability than whatever tool was automatically wrapping past the 85 character limit.

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

...crosoft.ML.Samples/Dynamic/Trainers/Recommendation/OneClassMatrixFactorizationWithOptions.cs

-                // the value of all missing matrix elements.
+                MatrixColumnIndexColumnName = nameof(
+                    MatrixElement.MatrixColumnIndex),
+                    MatrixRowIndexColumnName = nameof(MatrixElement.MatrixRowIndex),


📝 This is incorrectly indented

sharwell · 2019-07-02T14:32:38Z

...crosoft.ML.Samples/Dynamic/Trainers/Recommendation/OneClassMatrixFactorizationWithOptions.cs

+            // Apply the trained model to the test set. Notice that training is a
+            // partial 
+            var prediction = model.Transform(mlContext.Data.LoadFromEnumerable(
+                testData));


📝 Incorrect wrapping (prefer to wrap the outer argument when it will fit on the same number of lines)

sharwell · 2019-07-02T14:34:09Z

...mples/Microsoft.ML.Samples/Dynamic/Trainers/Recommendation/MatrixFactorizationWithOptions.tt

-				// Specify IDataView colum which stores matrix row indexes. 
+                // Specify IDataView colum which stores matrix column indexes. 
+                MatrixColumnIndexColumnName = nameof(MatrixElement.MatrixColumnIndex
+                    ),


📝 Incorrect wrapping (should be wrapped before nameof)

* Reformatted Recommendation samples * Fix for tab spacing errors * Removed precision for reformatting in Console.WriteLine and fixed KeyType error

sayanshaw24 added 2 commits July 1, 2019 12:15

Reformatted Recommendation samples

13b798f

Fix for tab spacing errors

3bbb568

sayanshaw24 requested review from codemzs, natke, wschin and artidoro July 1, 2019 19:31

natke approved these changes Jul 1, 2019

View reviewed changes

wschin reviewed Jul 1, 2019

View reviewed changes

...mples/Microsoft.ML.Samples/Dynamic/Trainers/Recommendation/MatrixFactorizationWithOptions.cs Show resolved Hide resolved

wschin reviewed Jul 1, 2019

View reviewed changes

...mples/Microsoft.ML.Samples/Dynamic/Trainers/Recommendation/MatrixFactorizationWithOptions.tt Show resolved Hide resolved

wschin reviewed Jul 1, 2019

View reviewed changes

Removed precision for reformatting in Console.WriteLine and fixed Key…

4715016

…Type error

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

codemzs approved these changes Jul 2, 2019

View reviewed changes

sayanshaw24 merged commit 01b3ec7 into dotnet:master Jul 2, 2019

sharwell reviewed Jul 2, 2019

View reviewed changes

sharwell requested changes Jul 2, 2019

View reviewed changes

sharwell reviewed Jul 2, 2019

View reviewed changes

sharwell mentioned this pull request Jul 2, 2019

Reformatting MulticlassClassification samples to width 85 #3942

Merged

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

Reformatted Recommendation samples to width 85 #3941

Reformatted Recommendation samples to width 85 #3941

sayanshaw24 commented Jul 1, 2019

wschin Jul 1, 2019

sayanshaw24 Jul 2, 2019

wschin Jul 1, 2019

sayanshaw24 Jul 1, 2019

wschin Jul 1, 2019 •

edited

Loading

wschin Jul 1, 2019

sayanshaw24 Jul 2, 2019 •

edited

Loading

codemzs left a comment

sharwell Jul 2, 2019 •

edited

Loading

sharwell Jul 2, 2019

sharwell Jul 2, 2019

sharwell left a comment

sharwell Jul 2, 2019

sharwell Jul 2, 2019

sharwell Jul 2, 2019

Reformatted Recommendation samples to width 85 #3941

Reformatted Recommendation samples to width 85 #3941

Conversation

sayanshaw24 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

wschin Jul 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sayanshaw24 Jul 2, 2019 • edited Loading

Choose a reason for hiding this comment

codemzs left a comment

Choose a reason for hiding this comment

sharwell Jul 2, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharwell 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

wschin Jul 1, 2019 •

edited

Loading

sayanshaw24 Jul 2, 2019 •

edited

Loading

sharwell Jul 2, 2019 •

edited

Loading