Conversion catalog samples #3167

sfilipi · 2019-04-02T06:26:55Z

Towards #1209
Adding and adjusting samples for the Conversions catalog.

codecov · 2019-04-02T07:05:12Z

Codecov Report

Merging #3167 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3167      +/-   ##
==========================================
- Coverage   72.54%   72.53%   -0.01%     
==========================================
  Files         807      807              
  Lines      144774   144774              
  Branches    16208    16208              
==========================================
- Hits       105022   105011      -11     
- Misses      35338    35348      +10     
- Partials     4414     4415       +1

Flag	Coverage Δ
#Debug	`72.53% <ø> (-0.01%)`	⬇️
#production	`68.12% <ø> (-0.01%)`	⬇️
#test	`88.82% <ø> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
...ML.Data/Transforms/ConversionsExtensionsCatalog.cs	`44.87% <ø> (ø)`	⬆️
...c/Microsoft.ML.FastTree/Utils/ThreadTaskManager.cs	`79.48% <0%> (-20.52%)`	⬇️
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs	`73.7% <0%> (-0.34%)`	⬇️

wschin · 2019-04-02T16:51:58Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValue.cs

+
+            public uint Label { get; set; }
+        }
+


3 empty lines? #Resolved

wschin · 2019-04-02T16:52:53Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertTypeMultiColumn.cs

+    public static class ConvertTypeMultiColumn
+    {
+        // The initial data type
+        private class InputData


Other places we have C# structures defined after the actual example function. #Resolved

i like that! focus should be on the example.

In reply to: 271401627 [](ancestors = 271401627)

wschin · 2019-04-02T16:53:20Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertTypeMultiColumn.cs

+        {
+            public float Converted1 { get; set; }
+            public float Converted2 { get; set; }
+


Do we want to remove this empty line? #Resolved

wschin · 2019-04-02T16:54:28Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertTypeMultiColumn.cs

+                    new InputOutputColumnPair("Converted3", "Feature3"),
+                    new InputOutputColumnPair("Converted4", "Feature4"),
+
+                }, DataKind.Single);


{ is not aligned with }. #Resolved

wschin · 2019-04-02T16:54:38Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertTypeMultiColumn.cs

+                    new InputOutputColumnPair("Converted2", "Feature2"),
+                    new InputOutputColumnPair("Converted3", "Feature3"),
+                    new InputOutputColumnPair("Converted4", "Feature4"),
+


Please remove this empty line. #Resolved

wschin · 2019-04-02T16:55:59Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertTypeMultiColumn.cs

+
+                }, DataKind.Single);
+
+            // Let's train our pipeline, and then apply it to the same data.


Please add more detailed description about what this pipeline is doing. For example, input column A will be converted to output column B. #Pending

I am not adding too many details, because it is hard to maintain as stale names in the comments don't get flagged by anything.

In reply to: 271402918 [](ancestors = 271402918)

wschin · 2019-04-02T16:56:29Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValue.cs

+{
+    public static class MapValue
+    {
+        class DataPoint


Suggested change

class DataPoint

private class DataPoint

``` #Resolved

wschin · 2019-04-02T16:56:42Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValue.cs

+            public int Score { get; set; }
+        }
+
+        class TransformedData : DataPoint


Suggested change

class TransformedData : DataPoint

private class TransformedDataPoint : DataPoint

``` #Resolved

wschin · 2019-04-02T16:57:56Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValue.cs

+
+
+        /// This example demonstrates the use of the ValueMappingEstimator by mapping strings to other string values, or floats to strings. 
+        /// This is useful to map types to a grouping. 


What does This is useful to map types to a grouping. mean? #Resolved

wschin · 2019-04-02T16:58:15Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValue.cs

+                new DataPoint() { Timeframe = "12-25yrs" , Score = 3 },
+                new DataPoint() { Timeframe = "0-5yrs" , Score = 4 },
+                new DataPoint() { Timeframe = "12-25yrs" , Score = 5 },
+                 new DataPoint() { Timeframe = "25+yrs" , Score = 5 },


Suggested change

new DataPoint() { Timeframe = "25+yrs" , Score = 5 },

new DataPoint() { Timeframe = "25+yrs" , Score = 5 },

``` #Resolved

wschin · 2019-04-02T16:58:40Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValue.cs

+
+        /// This example demonstrates the use of the ValueMappingEstimator by mapping strings to other string values, or floats to strings. 
+        /// This is useful to map types to a grouping. 
+        /// It is possible to have multiple values map to the same category.


This sentence is not very clear to me. #Resolved

zeahmed · 2019-04-02T17:24:14Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertTypeMultiColumn.cs

+namespace Microsoft.ML.Samples.Dynamic
+{
+    // This example illustrates how to convert multiple columns of different types to one type, in this case System.Single. 
+    // This is often a useful data transformation before concatenting the features together and passing them to a particular estimator.


concatenting [](start = 57, length = 12)

typo. #Closed

zeahmed · 2019-04-02T17:26:17Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertTypeMultiColumn.cs

+
+        public static void Example()
+        {
+            var mlContext = new MLContext(seed: 1);


var mlContext = new MLContext(seed: 1); [](start = 12, length = 39)

I see missing comments overall in this sample which we usually have in other samples e.g. comments above the MLContext etc. #Closed

zeahmed · 2019-04-02T17:33:36Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValue.cs

+            // TransformedData obtained post-transformation.
+            //
+            //  Timeframe   TimeframeCategory   Label    Score   ScoreCategory
+            // 0 - 4yrs         Short              1       1       Low


0 - 4 [](start = 15, length = 5)

There should not be spaces between hyphen here. #Closed

zeahmed · 2019-04-02T17:36:46Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValueIDVLookup.cs

+
+            // Create the lookup map data IEnumerable.   
+            var lookupData = new[] {
+                new LookupMap { Value = 3.14f, Category = "Low" },


Value = 3.14f, Category = "Low" [](start = 32, length = 31)

is it intentional that order of Value and Category differ from the rest? #Closed

zeahmed · 2019-04-02T17:37:37Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValueIDVLookup.cs

+            // Convert to IDataView
+            var lookupIdvMap = mlContext.Data.LoadFromEnumerable(lookupData);
+
+            // Constructs the ValueMappingEstimator making the ML.net pipeline


ML.net [](start = 63, length = 6)

I think we advertise it as ML.NET not ML.net, right? #Closed

zeahmed · 2019-04-02T17:39:49Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValueIDVLookup.cs

+            foreach (var featureRow in features)
+                Console.WriteLine($"{featureRow.Price}\t\t{featureRow.PriceCategory}");
+
+            // TransformedData obtained post-transformation.


// TransformedData obtained post-transformation. [](start = 12, length = 48)

Everywhere else this is "// Expected output:" #Closed

I think this is fine too, it matches the name of the type.

In reply to: 271420199 [](ancestors = 271420199)

ok.

In reply to: 271466196 [](ancestors = 271466196,271420199)

zeahmed · 2019-04-02T17:41:25Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValueToArray.cs

+            }
+
+            // Timeframe      Feature
+            // 0 - 4yrs       0, 5, 300


0 - 4yrs [](start = 15, length = 8)

No spaces between hyphen here. #Closed

zeahmed · 2019-04-02T17:42:27Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValueToArray.cs

+
+        class TransformedData : DataPoint
+        {
+            public int[] Feature { get; set; }


Feature [](start = 25, length = 7)

Should be 'Features" as you mentioned this as Features Column in the comments below, right? #Closed

zeahmed · 2019-04-02T17:45:05Z

src/Microsoft.ML.Data/Transforms/ConversionsExtensionsCatalog.cs

@@ -100,7 +106,7 @@ internal static TypeConvertingEstimator ConvertType(this TransformsCatalog.Conve
        /// <example>
        /// <format type="text/markdown">
        /// <![CDATA[
-        ///  [!code-csharp[KeyToValueMappingEstimator](~/../docs/samples/docs/samples/Microsoft.ML.Samples/Dynamic/ValueMappingStringToKeyType.cs)]
+        ///  [!code-csharp[KeyToValueMappingEstimator](~/../docs/samples/docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ValueMappingStringToKeyType.cs)]


KeyToValueMappingEstimator [](start = 27, length = 26)

It should be the name of the extension method e.g. MapKeyToValue. Can you please also check this inconsistency in the rest of the file? #Closed

shmoradims · 2019-04-02T18:19:36Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertTypeMultiColumn.cs

+        }
+
+        // The resulting data type after the transformation
+        private sealed class TransformedData : InputData


sealed [](start = 16, length = 6)

we don't need to worry about sealed. it might give the wrong impression that the data classes should be sealed.

I added this note to the checklist #Resolved

shmoradims · 2019-04-02T18:21:43Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertTypeMultiColumn.cs

+            public string Feature2;
+            public DateTime Feature3;
+            public double Feature4;
+        }


please move the data classes below example as discussed in the group (i also added this note to the checklist) #Resolved

shmoradims · 2019-04-02T18:33:08Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValueIDVLookup.cs

+                new DataPoint() { Price = 1.19f },
+                new DataPoint() { Price = 2.17f },
+                new DataPoint() { Price = 33.784f },
+


extra line #Resolved

shmoradims · 2019-04-02T18:33:23Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValueIDVLookup.cs

+            public string Category { get; set; }
+
+        }
+


extra line #Resolved

shmoradims · 2019-04-02T18:35:10Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/MapValueIDVLookup.cs

+
+namespace Microsoft.ML.Samples.Dynamic
+{
+    public static class MapValueIdvLookup


MapValueIdvLookup [](start = 24, length = 17)

Let's change the filename to say Idv instead of IDV #Resolved

shmoradims · 2019-04-02T18:59:20Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertTypeMultiColumn.cs

+            //      0        23         6.368921E+17        0.1206
+            //      1       8904        6.368924E+17        8.09
+
+        }


i see a lot of extra empty lines. please have a second pass and remove them. #Resolved

zeahmed

shmoradims · 2019-04-04T00:17:06Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertType.cs

@@ -51,5 +39,13 @@ public static void Example()
            // A: False    Aconv:0
            // A: False    Aconv:0
        }
+        private class InputData


pr [](start = 7, length = 3)

need extra line here

shmoradims · 2019-04-04T00:17:14Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/Conversion/ConvertType.cs

+        {
+            public bool Survived;
+        }
+        private sealed class TransformedData : InputData


pr [](start = 8, length = 2)

ditto

shmoradims

* adding a sample for convert MultiColumns. Moving files around. * Adjust the samples about ValueMapping * Addressing PR comments

sfilipi requested review from singlis and zeahmed April 2, 2019 06:26

sfilipi self-assigned this Apr 2, 2019

sfilipi added the documentation Related to documentation of ML.NET label Apr 2, 2019

sfilipi requested review from shmoradims, rogancarr and natke April 2, 2019 06:27

wschin reviewed Apr 2, 2019

View reviewed changes

sfilipi added 2 commits April 2, 2019 10:09

adding a sample for convert MultiColumns. Moving files around.

114fe15

Adjust the samples about ValueMapping

a51e688

zeahmed reviewed Apr 2, 2019

View reviewed changes

shmoradims reviewed Apr 2, 2019

View reviewed changes

Addressing PR comments

edbb1f0

sfilipi force-pushed the conversionSamples branch from 599a29a to edbb1f0 Compare April 2, 2019 20:29

zeahmed approved these changes Apr 2, 2019

View reviewed changes

shmoradims reviewed Apr 4, 2019

View reviewed changes

shmoradims approved these changes Apr 4, 2019

View reviewed changes

sfilipi merged commit e285889 into dotnet:master Apr 4, 2019

sfilipi deleted the conversionSamples branch April 4, 2019 16:49

sfilipi added a commit to sfilipi/machinelearning-1 that referenced this pull request Apr 9, 2019

Conversion catalog samples (dotnet#3167)

15e5976

* adding a sample for convert MultiColumns. Moving files around. * Adjust the samples about ValueMapping * Addressing PR comments

sfilipi mentioned this pull request Apr 9, 2019

Release cherry picks for Conversions samples and removing gplv2 files #3264

Closed

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


		}, DataKind.Single);

		// Let's train our pipeline, and then apply it to the same data.

	class TransformedData : DataPoint
	private class TransformedDataPoint : DataPoint
	``` #Resolved



		/// This example demonstrates the use of the ValueMappingEstimator by mapping strings to other string values, or floats to strings.
		/// This is useful to map types to a grouping.

	new DataPoint() { Timeframe = "25+yrs" , Score = 5 },
	new DataPoint() { Timeframe = "25+yrs" , Score = 5 },
	``` #Resolved

Conversion catalog samples #3167

Conversion catalog samples #3167

Conversation

sfilipi commented Apr 2, 2019

codecov bot commented Apr 2, 2019 • edited Loading

Codecov Report

wschin Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

wschin Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

sfilipi Apr 2, 2019

Choose a reason for hiding this comment

wschin Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

wschin Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

wschin Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

wschin Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

sfilipi Apr 2, 2019

Choose a reason for hiding this comment

wschin Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

wschin Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

wschin Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

wschin Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

wschin Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

zeahmed Apr 2, 2019 • edited Loading

Choose a reason for hiding this comment

zeahmed Apr 2, 2019 • edited Loading

Choose a reason for hiding this comment

zeahmed Apr 2, 2019 • edited Loading

Choose a reason for hiding this comment

zeahmed Apr 2, 2019 • edited Loading

Choose a reason for hiding this comment

zeahmed Apr 2, 2019 • edited Loading

Choose a reason for hiding this comment

zeahmed Apr 2, 2019 • edited Loading

Choose a reason for hiding this comment

sfilipi Apr 2, 2019

Choose a reason for hiding this comment

zeahmed Apr 2, 2019

Choose a reason for hiding this comment

zeahmed Apr 2, 2019 • edited Loading

Choose a reason for hiding this comment

zeahmed Apr 2, 2019 • edited Loading

Choose a reason for hiding this comment

zeahmed Apr 2, 2019 • edited Loading

Choose a reason for hiding this comment

shmoradims Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

shmoradims Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

shmoradims Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

shmoradims Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

shmoradims Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

shmoradims Apr 2, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

zeahmed left a comment

Choose a reason for hiding this comment

shmoradims Apr 4, 2019

Choose a reason for hiding this comment

shmoradims Apr 4, 2019

Choose a reason for hiding this comment

shmoradims left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 2, 2019 •

edited

Loading

wschin Apr 2, 2019 •

edited by sfilipi

Loading

wschin Apr 2, 2019 •

edited by sfilipi

Loading

wschin Apr 2, 2019 •

edited by sfilipi

Loading

wschin Apr 2, 2019 •

edited by sfilipi

Loading

wschin Apr 2, 2019 •

edited by sfilipi

Loading

wschin Apr 2, 2019 •

edited by sfilipi

Loading

wschin Apr 2, 2019 •

edited by sfilipi

Loading

wschin Apr 2, 2019 •

edited by sfilipi

Loading

wschin Apr 2, 2019 •

edited by sfilipi

Loading

wschin Apr 2, 2019 •

edited by sfilipi

Loading

wschin Apr 2, 2019 •

edited by sfilipi

Loading

zeahmed Apr 2, 2019 •

edited

Loading

zeahmed Apr 2, 2019 •

edited

Loading

zeahmed Apr 2, 2019 •

edited

Loading

zeahmed Apr 2, 2019 •

edited

Loading

zeahmed Apr 2, 2019 •

edited

Loading

zeahmed Apr 2, 2019 •

edited

Loading

zeahmed Apr 2, 2019 •

edited

Loading

zeahmed Apr 2, 2019 •

edited

Loading

zeahmed Apr 2, 2019 •

edited

Loading

shmoradims Apr 2, 2019 •

edited by sfilipi

Loading

shmoradims Apr 2, 2019 •

edited by sfilipi

Loading

shmoradims Apr 2, 2019 •

edited by sfilipi

Loading

shmoradims Apr 2, 2019 •

edited by sfilipi

Loading

shmoradims Apr 2, 2019 •

edited by sfilipi

Loading

shmoradims Apr 2, 2019 •

edited by sfilipi

Loading