review comments and some more cleanup

Author: artidoro

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

@@ -22,7 +22,8 @@ public static class ConversionsExtensionsCatalog
         /// </summary>
         /// <param name="catalog">The transform's catalog.</param>
         /// <param name="outputColumnName">Name of the column resulting from the transformation of <paramref name="inputColumnName"/>.</param>
-        /// <param name="inputColumnName">Name of the column to transform. If set to <see langword="null"/>, the value of the <paramref name="outputColumnName"/> will be used as source.</param>        /// <param name="hashBits">Number of bits to hash into. Must be between 1 and 31, inclusive.</param>
+        /// <param name="inputColumnName">Name of the column to transform. If set to <see langword="null"/>, the value of the <paramref name="outputColumnName"/> will be used as source.</param>
+        /// <param name="hashBits">Number of bits to hash into. Must be between 1 and 31, inclusive.</param>
         /// <param name="invertHash">During hashing we constuct mappings between original values and the produced hash values.
         /// Text representation of original values are stored in the slot names of the  metadata for the new column.Hashing, as such, can map many initial values to one.
         /// <paramref name="invertHash"/> specifies the upper bound of the number of distinct input values mapping to a hash that should be retained.

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

@@ -99,9 +99,9 @@ internal sealed class Options : TransformInputBase
         internal const string FriendlyName = "Feature Contribution Calculation";
         internal const string LoaderSignature = "FeatureContribution";
 
-        public readonly int Top;
-        public readonly int Bottom;
-        public readonly bool Normalize;
+        internal readonly int Top;
+        internal readonly int Bottom;
+        internal readonly bool Normalize;
 
         private readonly IFeatureContributionMapper _predictor;
 
@@ -281,7 +281,7 @@ public sealed class FeatureContributionCalculatingEstimator : TrivialEstimator<F
         private readonly string _featureColumn;
         private readonly ICalculateFeatureContribution _predictor;
 
-        public static class Defaults
+        internal static class Defaults
         {
             public const int NumPositiveContributions = 10;
             public const int NumNegativeContributions = 10;

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

@@ -53,7 +53,8 @@ public static CommonOutputs.TransformOutput Convert(IHostEnvironment env, TypeCo
     }
 
     /// <summary>
-    /// ConvertTransform allow to change underlying column type as long as we know how to convert types.
+    /// <see cref="TypeConvertingTransformer"/> converts underlying column types.
+    /// The source and destination column types need to be compatible.
     /// </summary>
     public sealed class TypeConvertingTransformer : OneToOneTransformerBase
     {
@@ -510,11 +511,12 @@ private bool SaveAsOnnxCore(OnnxContext ctx, int iinfo, string srcVariableName,
     }
 
     /// <summary>
-    /// Convert estimator allow you take column and change it type as long as we know how to do conversion between types.
+    /// <see cref="TypeConvertingEstimator"/> converts underlying column types.
+    /// The source and destination column types need to be compatible.
     /// </summary>
     public sealed class TypeConvertingEstimator : TrivialEstimator<TypeConvertingTransformer>
     {
-        public sealed class Defaults
+        internal sealed class Defaults
         {
             public const DataKind DefaultOutputKind = DataKind.R4;
         }
@@ -524,9 +526,21 @@ public sealed class Defaults
         /// </summary>
         public sealed class ColumnInfo
         {
+            /// <summary>
+            /// Name of the column resulting from the transformation of <see cref="InputColumnName"/>.
+            /// </summary>
             public readonly string Name;
+            /// <summary>
+            /// Name of column to transform. If set to <see langword="null"/>, the value of the <see cref="Name"/> will be used as source.
+            /// </summary>
             public readonly string InputColumnName;
+            /// <summary>
+            /// The expected kind of the converted column.
+            /// </summary>
             public readonly DataKind OutputKind;
+            /// <summary>
+            /// New key count, if we work with key type.
+            /// </summary>
             public readonly KeyCount OutputKeyCount;
 
             /// <summary>
@@ -535,14 +549,30 @@ public sealed class ColumnInfo
             /// <param name="name">Name of the column resulting from the transformation of <paramref name="inputColumnName"/>.</param>
             /// <param name="outputKind">The expected kind of the converted column.</param>
             /// <param name="inputColumnName">Name of column to transform. If set to <see langword="null"/>, the value of the <paramref name="name"/> will be used as source.</param>
-            /// <param name="outputKeyCount">New key range, if we work with key type.</param>
+            /// <param name="outputKeyCount">New key count, if we work with key type.</param>
             public ColumnInfo(string name, DataKind outputKind, string inputColumnName, KeyCount outputKeyCount = null)
             {
                 Name = name;
                 InputColumnName = inputColumnName ?? name;
                 OutputKind = outputKind;
                 OutputKeyCount = outputKeyCount;
             }
+
+            /// <summary>
+            /// Describes how the transformer handles one column pair.
+            /// </summary>
+            /// <param name="name">Name of the column resulting from the transformation of <paramref name="inputColumnName"/>.</param>
+            /// <param name="type">The expected kind of the converted column.</param>
+            /// <param name="inputColumnName">Name of column to transform. If set to <see langword="null"/>, the value of the <paramref name="name"/> will be used as source.</param>
+            /// <param name="outputKeyCount">New key count, if we work with key type.</param>
+            public ColumnInfo(string name, Type type, string inputColumnName, KeyCount outputKeyCount = null)
+            {
+                Name = name;
+                InputColumnName = inputColumnName ?? name;
+                if (!type.TryGetDataKind(out OutputKind))
+                    throw Contracts.ExceptUserArg(nameof(type), $"Unsupported type {type}.");
+                OutputKeyCount = outputKeyCount;
+            }
         }
 
         /// <summary>

src/Microsoft.ML.Transforms/CountFeatureSelection.cs

@@ -46,6 +46,9 @@ internal sealed class Options : TransformInputBase
 
         internal static string RegistrationName = "CountFeatureSelectionTransform";
 
+        /// <summary>
+        /// Describes how the transformer handles one column pair.
+        /// </summary>
         public sealed class ColumnInfo
         {
             public readonly string Name;
@@ -131,7 +134,7 @@ public SchemaShape GetOutputSchema(SchemaShape inputSchema)
         }
 
         /// <summary>
-        /// Train and return a transformer.
+        /// Trains and returns a <see cref="ITransformer"/>.
         /// </summary>
         public ITransformer Fit(IDataView input)
         {

src/Microsoft.ML.Transforms/MissingValueDroppingTransformer.cs

@@ -75,6 +75,9 @@ private static VersionInfo GetVersionInfo()
 
         private const string RegistrationName = "DropNAs";
 
+        /// <summary>
+        /// The names of the input columns of the transformation and the corresponding names for the output columns.
+        /// </summary>
         public IReadOnlyList<(string outputColumnName, string inputColumnName)> Columns => ColumnPairs.AsReadOnly();
 
         /// <summary>

src/Microsoft.ML.Transforms/MutualInformationFeatureSelection.cs

@@ -31,7 +31,8 @@ public sealed class MutualInformationFeatureSelectingEstimator : IEstimator<ITra
         internal const string ShortName = "MIFeatureSelection";
         internal static string RegistrationName = "MutualInformationFeatureSelectionTransform";
 
-        public static class Defaults
+        [BestFriend]
+        internal static class Defaults
         {
             public const string LabelColumn = DefaultColumnNames.Label;
             public const int SlotsInOutput = 1000;
@@ -116,7 +117,7 @@ internal MutualInformationFeatureSelectingEstimator(IHostEnvironment env, string
         }
 
         /// <summary>
-        /// Train and return a transformer.
+        /// Trains and returns a <see cref="ITransformer"/>.
         /// </summary>
         public ITransformer Fit(IDataView input)
         {

test/Microsoft.ML.Tests/Transformers/ConvertTests.cs

@@ -78,7 +78,7 @@ public void TestConvertWorkout()
             var data = new[] { new TestClass() { A = 1, B = new int[2] { 1,4 } },
                                new TestClass() { A = 2, B = new int[2] { 3,4 } }};
             var dataView = ML.Data.ReadFromEnumerable(data);
-            var pipe = new TypeConvertingEstimator(Env, columns: new[] {new TypeConvertingEstimator.ColumnInfo("ConvA", DataKind.R4, "A"),
+            var pipe = ML.Transforms.Conversion.ConvertType(columns: new[] {new TypeConvertingEstimator.ColumnInfo("ConvA", DataKind.R4, "A"),
                 new TypeConvertingEstimator.ColumnInfo("ConvB", DataKind.R4, "B")});
 
             TestEstimatorCore(pipe, dataView);
@@ -117,7 +117,7 @@ public void TestConvertWorkout()
             };
 
             var allTypesDataView = ML.Data.ReadFromEnumerable(allTypesData);
-            var allTypesPipe = new TypeConvertingEstimator(Env, columns: new[] {
+            var allTypesPipe = ML.Transforms.Conversion.ConvertType(columns: new[] {
                 new TypeConvertingEstimator.ColumnInfo("ConvA", DataKind.R4, "AA"),
                 new TypeConvertingEstimator.ColumnInfo("ConvB", DataKind.R4, "AB"),
                 new TypeConvertingEstimator.ColumnInfo("ConvC", DataKind.R4, "AC"),
@@ -192,8 +192,8 @@ public void TestOldSavingAndLoading()
             var data = new[] { new TestClass() { A = 1, B = new int[2] { 1,4 } },
                                new TestClass() { A = 2, B = new int[2] { 3,4 } }};
             var dataView = ML.Data.ReadFromEnumerable(data);
-            var pipe = new TypeConvertingEstimator(Env, columns: new[] {new TypeConvertingEstimator.ColumnInfo("ConvA", DataKind.R8, "A"),
-                new TypeConvertingEstimator.ColumnInfo("ConvB", DataKind.R8, "B")});
+            var pipe = ML.Transforms.Conversion.ConvertType(columns: new[] {new TypeConvertingEstimator.ColumnInfo("ConvA", typeof(double), "A"),
+                new TypeConvertingEstimator.ColumnInfo("ConvB", typeof(double), "B")});
 
             var result = pipe.Fit(dataView).Transform(dataView);
             var resultRoles = new RoleMappedData(result);
@@ -213,7 +213,7 @@ public void TestMetadata()
             var pipe = new OneHotEncodingEstimator(Env, new[] {
                 new OneHotEncodingEstimator.ColumnInfo("CatA", "A", OneHotEncodingTransformer.OutputKind.Ind),
                 new OneHotEncodingEstimator.ColumnInfo("CatB", "B", OneHotEncodingTransformer.OutputKind.Key)
-            }).Append(new TypeConvertingEstimator(Env, new[] {
+            }).Append(ML.Transforms.Conversion.ConvertType(new[] {
                 new TypeConvertingEstimator.ColumnInfo("ConvA", DataKind.R8, "CatA"),
                 new TypeConvertingEstimator.ColumnInfo("ConvB", DataKind.U2, "CatB")
             }));