Cleaning our attributes classes (#2638)

Author: Ivanidzo4ka

docs/samples/Microsoft.ML.Samples/Dynamic/TensorFlow/ImageClassification.cs

@@ -2,7 +2,7 @@
 using System.Linq;
 using Microsoft.ML.Data;
 
-namespace Microsoft.ML.Samples.Dynamic.TensorFlow
+namespace Microsoft.ML.Samples.Dynamic
 {
     public static class ImageClassification
     {

docs/samples/Microsoft.ML.Samples/Dynamic/TensorFlow/TextClassification.cs

@@ -3,7 +3,7 @@
 using Microsoft.ML.Data;
 using Microsoft.ML.Transforms.TensorFlow;
 
-namespace Microsoft.ML.Samples.Dynamic.TensorFlow
+namespace Microsoft.ML.Samples.Dynamic
 {
     public static class TextClassification
     {
@@ -103,11 +103,11 @@ public class IMDBSentiment
             public string Sentiment_Text { get; set; }
 
             /// <summary>
-            /// This is a variable length vector designated by VectorType(0) attribute.
+            /// This is a variable length vector designated by VectorType attribute.
             /// Variable length vectors are produced by applying operations such as 'TokenizeWords' on strings
             /// resulting in vectors of tokens of variable lengths.
             /// </summary>
-            [VectorType(0)]
+            [VectorType]
             public int[] VariableLenghtFeatures { get; set; }
         }
 

src/Microsoft.ML.Core/CommandLine/DefaultArgumentAttribute.cs

@@ -15,7 +15,7 @@ namespace Microsoft.ML.CommandLine
     /// </summary>
     [AttributeUsage(AttributeTargets.Field)]
     [BestFriend]
-    internal class DefaultArgumentAttribute : ArgumentAttribute
+    internal sealed class DefaultArgumentAttribute : ArgumentAttribute
     {
         /// <summary>
         /// Indicates that this argument is the default argument.

src/Microsoft.ML.Core/CommandLine/EnumValueDisplayAttribute.cs

@@ -11,7 +11,7 @@ namespace Microsoft.ML.CommandLine
     /// </summary>
     [AttributeUsage(AttributeTargets.Field)]
     [BestFriend]
-    internal class EnumValueDisplayAttribute : Attribute
+    internal sealed class EnumValueDisplayAttribute : Attribute
     {
         public readonly string Name;
 

src/Microsoft.ML.Core/CommandLine/HideEnumValueAttribute.cs

@@ -11,7 +11,7 @@ namespace Microsoft.ML.CommandLine
     /// </summary>
     [AttributeUsage(AttributeTargets.Field)]
     [BestFriend]
-    internal class HideEnumValueAttribute : Attribute
+    internal sealed class HideEnumValueAttribute : Attribute
     {
         public HideEnumValueAttribute()
         {

src/Microsoft.ML.Data/Data/SchemaDefinition.cs

@@ -12,82 +12,108 @@
 namespace Microsoft.ML.Data
 {
     /// <summary>
-    /// Attach to a member of a class to indicate that the item type should be of class key.
+    /// Allow member to be marked as a <see cref="KeyType"/>.
     /// </summary>
+    /// <remarks>
+    /// Can be applied only for member of following types: <see cref="byte"/>, <see cref="ushort"/>, <see cref="uint"/>, <see cref="ulong"/>
+    /// </remarks>
     [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
     public sealed class KeyTypeAttribute : Attribute
     {
-        // REVIEW: Property based, but should I just have a constructor?
+        /// <summary>
+        /// Marks member as <see cref="KeyType"/>.
+        /// </summary>
+        /// <remarks>
+        /// Cardinality of <see cref="KeyType"/> would be maximum legal value of member type.
+        /// </remarks>
+        public KeyTypeAttribute()
+        {
+
+        }
+
+        /// <summary>
+        /// Marks member as <see cref="KeyType"/> and specifies <see cref="KeyType"/> cardinality.
+        /// </summary>
+        /// <param name="count">Cardinality of <see cref="KeyType"/>.</param>
+        public KeyTypeAttribute(ulong count)
+        {
+            KeyCount = new KeyCount(count);
+        }
 
         /// <summary>
         /// The key count.
         /// </summary>
-        public ulong Count { get; set; }
+        internal KeyCount KeyCount { get; }
     }
 
     /// <summary>
-    /// Allows a member to be marked as a vector valued field, primarily allowing one to set
+    /// Allows a member to be marked as a <see cref="VectorType"/>, primarily allowing one to set
     /// the dimensionality of the resulting array.
     /// </summary>
     [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
     public sealed class VectorTypeAttribute : Attribute
     {
-        private readonly int[] _dims;
-
         /// <summary>
         /// The length of the vectors from this vector valued field.
         /// </summary>
-        public int[] Dims { get { return _dims; } }
+        internal int[] Dims { get; }
 
-        public VectorTypeAttribute(params int[] dims)
+        /// <summary>
+        /// Mark member as single-dimensional array with unknown size.
+        /// </summary>
+        public VectorTypeAttribute()
         {
-            _dims = dims;
-        }
-    }
 
-    /// <summary>
-    /// Describes column information such as name and the source columns indicies that this
-    /// column encapsulates.
-    /// </summary>
-    [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
-    public sealed class ColumnAttribute : Attribute
-    {
-        public ColumnAttribute(string ordinal, string name = null)
+        }
+        /// <summary>
+        /// Mark member as single-dimensional array with specified size.
+        /// </summary>
+        /// <param name="size">Expected size of array. A zero value indicates that the vector type is considered to have unknown length.</param>
+        public VectorTypeAttribute(int size)
         {
-            Name = name;
+            Contracts.CheckParam(size >= 0, nameof(size), "Should be non-negative number");
+            Dims = new int[1] { size };
         }
 
         /// <summary>
-        /// Column name.
+        /// Mark member with expected dimensions of array.
         /// </summary>
-        public string Name { get; }
+        /// <param name="dimensions">Dimensions of array. All values should be non-negative.
+        /// A zero value indicates that the vector type is considered to have unknown length along that dimension.</param>
+        public VectorTypeAttribute(params int[] dimensions)
+        {
+            foreach (var size in dimensions)
+            {
+                Contracts.CheckParam(size >= 0, nameof(dimensions), "Should contain only non-negative values");
+            }
+            Dims = dimensions;
+        }
     }
 
     /// <summary>
-    /// Allows a member to specify its column name directly, as opposed to the default
+    /// Allows a member to specify <see cref="IDataView"/> column name directly, as opposed to the default
     /// behavior of using the member name as the column name.
     /// </summary>
     [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
     public sealed class ColumnNameAttribute : Attribute
     {
-        private readonly string _name;
         /// <summary>
         /// Column name.
         /// </summary>
-        public string Name { get { return _name; } }
+        internal string Name { get; }
 
         /// <summary>
-        /// Allows one to specify a name to expose this column as, as opposed to simply
-        /// the field name.
+        /// Allows one to specify a name to expose this column as, as opposed to the default
+        /// behavior of using the member name as the column name.
         /// </summary>
         public ColumnNameAttribute(string name)
         {
-            _name = name;
+            Name = name;
         }
     }
 
     /// <summary>
-    /// Mark this member as not being exposed as a column in the schema.
+    /// Mark this member as not being exposed as a <see cref="IDataView"/> column in the <see cref="DataViewSchema"/>.
     /// </summary>
     [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
     public sealed class NoColumnAttribute : Attribute
@@ -362,9 +388,8 @@ public static SchemaDefinition Create(Type userType, Direction direction = Direc
                 if (memberInfo.GetCustomAttribute<NoColumnAttribute>() != null)
                     continue;
 
-                var mappingAttr = memberInfo.GetCustomAttribute<ColumnAttribute>();
                 var mappingNameAttr = memberInfo.GetCustomAttribute<ColumnNameAttribute>();
-                string name = mappingAttr?.Name ?? mappingNameAttr?.Name ?? memberInfo.Name;
+                string name = mappingNameAttr?.Name ?? memberInfo.Name;
                 // Disallow duplicate names, because the field enumeration order is not actually
                 // well defined, so we are not gauranteed to have consistent "hiding" from run to
                 // run, across different .NET versions.
@@ -379,7 +404,10 @@ public static SchemaDefinition Create(Type userType, Direction direction = Direc
                 {
                     if (!KeyType.IsValidDataType(dataType))
                         throw Contracts.ExceptParam(nameof(userType), "Member {0} marked with KeyType attribute, but does not appear to be a valid kind of data for a key type", memberInfo.Name);
-                    itemType = new KeyType(dataType, keyAttr.Count);
+                    if (keyAttr.KeyCount == null)
+                        itemType = new KeyType(dataType, dataType.ToMaxInt());
+                    else
+                        itemType = new KeyType(dataType, keyAttr.KeyCount.Count.GetValueOrDefault());
                 }
                 else
                     itemType = ColumnTypeExtensions.PrimitiveTypeFromType(dataType);
@@ -388,7 +416,7 @@ public static SchemaDefinition Create(Type userType, Direction direction = Direc
                 DataViewType columnType;
                 var vectorAttr = memberInfo.GetCustomAttribute<VectorTypeAttribute>();
                 if (vectorAttr != null && !isVector)
-                    throw Contracts.ExceptParam(nameof(userType), "Member {0} marked with VectorType attribute, but does not appear to be a vector type", memberInfo.Name);
+                    throw Contracts.ExceptParam(nameof(userType), $"Member {memberInfo.Name} marked with {nameof(VectorTypeAttribute)}, but does not appear to be a vector type", memberInfo.Name);
                 if (isVector)
                 {
                     int[] dims = vectorAttr?.Dims;

src/Microsoft.ML.Data/DataLoadSave/Text/LoadColumnAttribute.cs

@@ -4,41 +4,42 @@
 
 using System;
 using System.Collections.Generic;
+using Microsoft.Data.DataView;
 
 namespace Microsoft.ML.Data
 {
     /// <summary>
-    /// Describes column information such as name and the source columns indices that this
-    /// column encapsulates.
+    /// Allow member to specify mapping to field(s) in text file.
+    /// To override name of <see cref="IDataView"/> column use <see cref="ColumnNameAttribute"/>.
     /// </summary>
     [AttributeUsage(AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = true)]
     public sealed class LoadColumnAttribute : Attribute
     {
         /// <summary>
-        /// Initializes new instance of <see cref="LoadColumnAttribute"/>.
+        /// Maps member to specific field in text file.
         /// </summary>
-        /// <param name="columnIndex">The index of the column in the text file.</param>
-        public LoadColumnAttribute(int columnIndex)
+        /// <param name="fieldIndex">The index of the field in the text file.</param>
+        public LoadColumnAttribute(int fieldIndex)
         {
             Sources = new List<TextLoader.Range>();
-            Sources.Add(new TextLoader.Range(columnIndex));
+            Sources.Add(new TextLoader.Range(fieldIndex));
         }
 
         /// <summary>
-        /// Initializes new instance of <see cref="LoadColumnAttribute"/>.
+        /// Maps member to range of fields in text file.
         /// </summary>
-        /// <param name="start">The starting column index, for the range.</param>
-        /// <param name="end">The ending column index, for the range.</param>
+        /// <param name="start">The starting field index, for the range.</param>
+        /// <param name="end">The ending field index, for the range.</param>
         public LoadColumnAttribute(int start, int end)
         {
             Sources = new List<TextLoader.Range>();
             Sources.Add(new TextLoader.Range(start, end));
         }
 
         /// <summary>
-        /// Initializes new instance of <see cref="LoadColumnAttribute"/>.
+        /// Maps member to set of fields in text file.
         /// </summary>
-        /// <param name="columnIndexes">Distinct text file column indices to load as part of this column.</param>
+        /// <param name="columnIndexes">Distinct text file field indices to load as part of this column.</param>
         public LoadColumnAttribute(int[] columnIndexes)
         {
             Sources = new List<TextLoader.Range>();

src/Microsoft.ML.Data/Depricated/TGUIAttribute.cs

@@ -10,7 +10,7 @@ namespace Microsoft.ML.Internal.Internallearn
     // REVIEW: Get rid of this. Everything should be in the ArgumentAttribute (or a class
     // derived from ArgumentAttribute).
     [AttributeUsage(AttributeTargets.Field)]
-    public class TGUIAttribute : Attribute
+    internal sealed class TGUIAttribute : Attribute
 #pragma warning restore MSML_GeneralName
     {
         // Display parameters

src/Microsoft.ML.SamplesUtils/SamplesDatasetUtils.cs

@@ -643,19 +643,19 @@ public static List<MulticlassClassificationExample> GenerateRandomMulticlassClas
         // and MatrixRowIndex=0 in MatrixElement below specifies the value at the upper-left corner in the training matrix). If user's row index
         // starts with 1, their row index 1 would be mapped to the 2nd row in matrix factorization module and their first row may contain no values.
         // This behavior is also true to column index.
-        private const int _synthesizedMatrixFirstColumnIndex = 1;
-        private const int _synthesizedMatrixFirstRowIndex = 1;
-        private const int _synthesizedMatrixColumnCount = 60;
-        private const int _synthesizedMatrixRowCount = 100;
+        private const uint _synthesizedMatrixFirstColumnIndex = 1;
+        private const uint _synthesizedMatrixFirstRowIndex = 1;
+        private const uint _synthesizedMatrixColumnCount = 60;
+        private const uint _synthesizedMatrixRowCount = 100;
 
         // A data structure used to encode a single value in matrix
         public class MatrixElement
         {
             // Matrix column index is at most _synthesizedMatrixColumnCount + _synthesizedMatrixFirstColumnIndex.
-            [KeyType(Count = _synthesizedMatrixColumnCount + _synthesizedMatrixFirstColumnIndex)]
+            [KeyType(_synthesizedMatrixColumnCount + _synthesizedMatrixFirstColumnIndex)]
             public uint MatrixColumnIndex;
             // Matrix row index is at most _synthesizedMatrixRowCount + _synthesizedMatrixFirstRowIndex.
-            [KeyType(Count = _synthesizedMatrixRowCount + _synthesizedMatrixFirstRowIndex)]
+            [KeyType(_synthesizedMatrixRowCount + _synthesizedMatrixFirstRowIndex)]
             public uint MatrixRowIndex;
             // The value at the column MatrixColumnIndex and row MatrixRowIndex.
             public float Value;
@@ -665,9 +665,9 @@ public class MatrixElement
         // renamed to Score because Score is the default name of matrix factorization's output.
         public class MatrixElementForScore
         {
-            [KeyType(Count = _synthesizedMatrixColumnCount + _synthesizedMatrixFirstColumnIndex)]
+            [KeyType(_synthesizedMatrixColumnCount + _synthesizedMatrixFirstColumnIndex)]
             public uint MatrixColumnIndex;
-            [KeyType(Count = _synthesizedMatrixRowCount + _synthesizedMatrixFirstRowIndex)]
+            [KeyType(_synthesizedMatrixRowCount + _synthesizedMatrixFirstRowIndex)]
             public uint MatrixRowIndex;
             public float Score;
         }

test/Microsoft.ML.Benchmarks/Helpers/CIBenchmark.cs

@@ -11,7 +11,7 @@ namespace Microsoft.ML.Benchmarks.Harness
     /// which we want to run on the CI.
     /// </summary>
     [AttributeUsage(AttributeTargets.Class, AllowMultiple = false)]
-    public class CIBenchmark : Attribute
+    public sealed class CIBenchmark : Attribute
     {
     }
 }

test/Microsoft.ML.Benchmarks/PredictionEngineBench.cs

@@ -168,10 +168,10 @@ public class SentimentPrediction
 
     public class BreastCancerData
     {
-        [ColumnName("Label"), Column("0")]
+        [ColumnName("Label")]
         public bool Label;
 
-        [ColumnName("Features"), Column("1-9"), VectorType(9)]
+        [ColumnName("Features"), VectorType(9)]
         public float[] Features;
     }
 

test/Microsoft.ML.FSharp.Tests/SmokeTests.fs

@@ -61,9 +61,9 @@ open Xunit
 module SmokeTest1 = 
 
     type SentimentData() =
-        [<LoadColumn(columnIndex = 0); ColumnName("Label"); DefaultValue>]
+        [<LoadColumn(fieldIndex = 0); ColumnName("Label"); DefaultValue>]
         val mutable Sentiment : bool
-        [<LoadColumn(columnIndex = 1); DefaultValue>]
+        [<LoadColumn(fieldIndex =1); DefaultValue>]
         val mutable SentimentText : string
 
     type SentimentPrediction() =
@@ -99,10 +99,10 @@ module SmokeTest2 =
 
     [<CLIMutable>]
     type SentimentData =
-        { [<LoadColumn(columnIndex = 0); ColumnName("Label")>] 
+        { [<LoadColumn(fieldIndex = 0); ColumnName("Label")>] 
           Sentiment : bool
 
-          [<LoadColumn(columnIndex = 1)>] 
+          [<LoadColumn(fieldIndex = 1)>] 
           SentimentText : string }
 
     [<CLIMutable>]
@@ -137,10 +137,10 @@ module SmokeTest2 =
 module SmokeTest3 = 
 
     type SentimentData() =
-        [<LoadColumn(columnIndex = 0); ColumnName("Label")>] 
+        [<LoadColumn(fieldIndex = 0); ColumnName("Label")>] 
         member val Sentiment = false with get, set
 
-        [<LoadColumn(columnIndex = 1)>] 
+        [<LoadColumn(fieldIndex = 1)>] 
         member val SentimentText = "".AsMemory() with get, set
 
     type SentimentPrediction() =

test/Microsoft.ML.Tests/ScenariosWithDirectInstantiation/TensorflowTests.cs

@@ -824,7 +824,6 @@ private int GetMaxIndexForOnePrediction(MNISTPrediction onePrediction)
 
         public class MNISTData
         {
-            [Column("0")]
             public long Label;
 
             [VectorType(784)]