Remove "VectorType" specific members on ColumnType. (#2131)

* Remove "VectorType" specific members on ColumnType.

Remove the following members from ColumnType:

- IsVector
- ItemType
- IsKnownSizeVector
- VectorSize
- ValueCount

Part of the work necessary for #1860 and contributes to #1533.

* Address review comments.

- Make extension methods verbs.
- Add doc to GetItemType extension.
- Fix one place using Size > 0 => IsKnownSize.

Author: eerhardt

src/Microsoft.ML.Core/Data/ColumnType.cs

@@ -18,17 +18,10 @@ namespace Microsoft.ML.Data
     /// </summary>
     public abstract class ColumnType : IEquatable<ColumnType>
     {
-        // This private constructor sets all the IsXxx flags. It is invoked by other ctors.
-        private ColumnType()
-        {
-            IsVector = this is VectorType;
-        }
-
         /// <summary>
         /// Constructor for extension types, which must be either <see cref="PrimitiveType"/> or <see cref="StructuredType"/>.
         /// </summary>
         private protected ColumnType(Type rawType)
-            : this()
         {
             Contracts.CheckValue(rawType, nameof(rawType));
             RawType = rawType;
@@ -41,7 +34,6 @@ private protected ColumnType(Type rawType)
         /// This asserts that they are consistent.
         /// </summary>
         private protected ColumnType(Type rawType, DataKind rawKind)
-            : this()
         {
             Contracts.AssertValue(rawType);
 #if DEBUG
@@ -70,80 +62,12 @@ private protected ColumnType(Type rawType, DataKind rawKind)
         [BestFriend]
         internal DataKind RawKind { get; }
 
-        /// <summary>
-        /// Whether this is a vector type. External code should just check directly against whether this type
-        /// is <see cref="VectorType"/>.
-        /// </summary>
-        [BestFriend]
-        internal bool IsVector { get; }
-
-        /// <summary>
-        /// For non-vector types, this returns the column type itself (i.e., return <c>this</c>).
-        /// </summary>
-        [BestFriend]
-        internal ColumnType ItemType => ItemTypeCore;
-
-        /// <summary>
-        /// Whether this is a vector type with known size. Returns false for non-vector types.
-        /// Equivalent to <c><see cref="VectorSize"/> &gt; 0</c>.
-        /// </summary>
-        [BestFriend]
-        internal bool IsKnownSizeVector => VectorSize > 0;
-
-        /// <summary>
-        /// Zero return means either it's not a vector or the size is unknown.
-        /// </summary>
-        [BestFriend]
-        internal int VectorSize => VectorSizeCore;
-
-        /// <summary>
-        /// For non-vectors, this returns one. For unknown size vectors, it returns zero.
-        /// Equivalent to IsVector ? VectorSize : 1.
-        /// </summary>
-        [BestFriend]
-        internal int ValueCount => ValueCountCore;
-
-        /// <summary>
-        /// The only sub-class that should override this is VectorType!
-        /// </summary>
-        private protected virtual ColumnType ItemTypeCore => this;
-
-        /// <summary>
-        /// The only sub-class that should override this is <see cref="VectorType"/>!
-        /// </summary>
-        private protected virtual int VectorSizeCore => 0;
-
-        /// <summary>
-        /// The only sub-class that should override this is VectorType!
-        /// </summary>
-        private protected virtual int ValueCountCore => 1;
-
         // IEquatable<T> interface recommends also to override base class implementations of
         // Object.Equals(Object) and GetHashCode. In classes below where Equals(ColumnType other)
         // is effectively a referencial comparison, there is no need to override base class implementations
         // of Object.Equals(Object) (and GetHashCode) since its also a referencial comparison.
         public abstract bool Equals(ColumnType other);
 
-        /// <summary>
-        /// Equivalent to calling Equals(ColumnType) for non-vector types. For vector type,
-        /// returns true if current and other vector types have the same size and item type.
-        /// </summary>
-        [BestFriend]
-        internal bool SameSizeAndItemType(ColumnType other)
-        {
-            if (other == null)
-                return false;
-
-            if (Equals(other))
-                return true;
-
-            // For vector types, we don't care about the factoring of the dimensions.
-            if (!IsVector || !other.IsVector)
-                return false;
-            if (!ItemType.Equals(other.ItemType))
-                return false;
-            return VectorSize == other.VectorSize;
-        }
     }
 
     /// <summary>
@@ -788,25 +712,25 @@ private static int ComputeSize(ImmutableArray<int> dims)
             return size;
         }
 
+        /// <summary>
+        /// Whether this is a vector type with known size.
+        /// Equivalent to <c><see cref="Size"/> &gt; 0</c>.
+        /// </summary>
+        public bool IsKnownSize => Size > 0;
+
         /// <summary>
         /// The type of the items stored as values in vectors of this type.
         /// </summary>
-        public new PrimitiveType ItemType { get; }
+        public PrimitiveType ItemType { get; }
 
         /// <summary>
         /// The size of the vector. A value of zero means it is a vector whose size is unknown.
         /// A vector whose size is known should correspond to values that always have the same <see cref="VBuffer{T}.Length"/>,
-        /// whereas one whose size is known may have values whose <see cref="VBuffer{T}.Length"/> varies from record to record.
+        /// whereas one whose size is unknown may have values whose <see cref="VBuffer{T}.Length"/> varies from record to record.
         /// Note that this is always the product of the elements in <see cref="Dimensions"/>.
         /// </summary>
         public int Size { get; }
 
-        private protected override ColumnType ItemTypeCore => ItemType;
-
-        private protected override int VectorSizeCore => Size;
-
-        private protected override int ValueCountCore => Size;
-
         public override bool Equals(ColumnType other)
         {
             if (other == this)

src/Microsoft.ML.Core/Data/ColumnTypeExtensions.cs

@@ -22,5 +22,48 @@ public static bool IsStandardScalar(this ColumnType columnType) =>
         /// Zero return means either it's not a key type or the cardinality is unknown.
         /// </summary>
         public static int GetKeyCount(this ColumnType columnType) => (columnType as KeyType)?.Count ?? 0;
+
+        /// <summary>
+        /// For non-vector types, this returns the column type itself (i.e., return <paramref name="columnType"/>).
+        /// For vector types, this returns the type of the items stored as values in vector.
+        /// </summary>
+        public static ColumnType GetItemType(this ColumnType columnType) => (columnType as VectorType)?.ItemType ?? columnType;
+
+        /// <summary>
+        /// Zero return means either it's not a vector or the size is unknown.
+        /// </summary>
+        public static int GetVectorSize(this ColumnType columnType) => (columnType as VectorType)?.Size ?? 0;
+
+        /// <summary>
+        /// For non-vectors, this returns one. For unknown size vectors, it returns zero.
+        /// For known sized vectors, it returns size.
+        /// </summary>
+        public static int GetValueCount(this ColumnType columnType) => (columnType as VectorType)?.Size ?? 1;
+
+        /// <summary>
+        /// Whether this is a vector type with known size. Returns false for non-vector types.
+        /// Equivalent to <c><see cref="GetVectorSize"/> &gt; 0</c>.
+        /// </summary>
+        public static bool IsKnownSizeVector(this ColumnType columnType) => columnType.GetVectorSize() > 0;
+
+        /// <summary>
+        /// Equivalent to calling Equals(ColumnType) for non-vector types. For vector type,
+        /// returns true if current and other vector types have the same size and item type.
+        /// </summary>
+        public static bool SameSizeAndItemType(this ColumnType columnType, ColumnType other)
+        {
+            if (other == null)
+                return false;
+
+            if (columnType.Equals(other))
+                return true;
+
+            // For vector types, we don't care about the factoring of the dimensions.
+            if (!(columnType is VectorType vectorType) || !(other is VectorType otherVectorType))
+                return false;
+            if (!vectorType.ItemType.Equals(otherVectorType.ItemType))
+                return false;
+            return otherVectorType.Size == otherVectorType.Size;
+        }
     }
 }

src/Microsoft.ML.Core/Data/IEstimator.cs

@@ -63,7 +63,7 @@ internal Column(string name, VectorKind vecKind, ColumnType itemType, bool isKey
                 Contracts.CheckNonEmpty(name, nameof(name));
                 Contracts.CheckValueOrNull(metadata);
                 Contracts.CheckParam(!(itemType is KeyType), nameof(itemType), "Item type cannot be a key");
-                Contracts.CheckParam(!itemType.IsVector, nameof(itemType), "Item type cannot be a vector");
+                Contracts.CheckParam(!(itemType is VectorType), nameof(itemType), "Item type cannot be a vector");
                 Contracts.CheckParam(!isKey || KeyType.IsValidDataKind(itemType.RawKind), nameof(itemType), "The item type must be valid for a key");
 
                 Name = name;
@@ -146,17 +146,28 @@ internal static void GetColumnTypeShape(ColumnType type,
             out ColumnType itemType,
             out bool isKey)
         {
-            if (type.IsKnownSizeVector)
-                vecKind = Column.VectorKind.Vector;
-            else if (type.IsVector)
-                vecKind = Column.VectorKind.VariableVector;
+            if (type is VectorType vectorType)
+            {
+                if (vectorType.IsKnownSize)
+                {
+                    vecKind = Column.VectorKind.Vector;
+                }
+                else
+                {
+                    vecKind = Column.VectorKind.VariableVector;
+                }
+
+                itemType = vectorType.ItemType;
+            }
             else
+            {
                 vecKind = Column.VectorKind.Scalar;
+                itemType = type;
+            }
 
-            itemType = type.ItemType;
-            isKey = type.ItemType is KeyType;
+            isKey = itemType is KeyType;
             if (isKey)
-                itemType = PrimitiveType.FromKind(type.ItemType.RawKind);
+                itemType = PrimitiveType.FromKind(itemType.RawKind);
         }
 
         /// <summary>

src/Microsoft.ML.Core/Data/MetadataUtils.cs

@@ -300,7 +300,9 @@ internal static IEnumerable<int> GetColumnSet(this Schema schema, string metadat
         ///  * metadata type is VBuffer&lt;ReadOnlyMemory&lt;char&gt;&gt; of length N
         /// </summary>
         public static bool HasSlotNames(this Schema.Column column)
-            => column.Type.IsKnownSizeVector && column.HasSlotNames(column.Type.VectorSize);
+            => column.Type is VectorType vectorType
+                && vectorType.Size > 0
+                && column.HasSlotNames(vectorType.Size);
 
         /// <summary>
         /// Returns <c>true</c> if the specified column:
@@ -316,9 +318,9 @@ internal static bool HasSlotNames(this Schema.Column column, int vectorSize)
             var metaColumn = column.Metadata.Schema.GetColumnOrNull(Kinds.SlotNames);
             return
                 metaColumn != null
-                && metaColumn.Value.Type.IsVector
-                && metaColumn.Value.Type.VectorSize == vectorSize
-                && metaColumn.Value.Type.ItemType is TextType;
+                && metaColumn.Value.Type is VectorType vectorType
+                && vectorType.Size == vectorSize
+                && vectorType.ItemType is TextType;
         }
 
         public static void GetSlotNames(this Schema.Column column, ref VBuffer<ReadOnlyMemory<char>> slotNames)
@@ -346,9 +348,9 @@ internal static bool HasKeyValues(this Schema.Column column, int keyCount)
             var metaColumn = column.Metadata.Schema.GetColumnOrNull(Kinds.KeyValues);
             return
                 metaColumn != null
-                && metaColumn.Value.Type.IsVector
-                && metaColumn.Value.Type.VectorSize == keyCount
-                && metaColumn.Value.Type.ItemType is TextType;
+                && metaColumn.Value.Type is VectorType vectorType
+                && vectorType.Size == keyCount
+                && vectorType.ItemType is TextType;
         }
 
         [BestFriend]

src/Microsoft.ML.Data/Commands/ShowSchemaCommand.cs

@@ -149,18 +149,18 @@ private static void PrintSchema(TextWriter writer, Arguments args, Schema schema
 
                     if (!args.ShowSlots)
                         continue;
-                    if (!type.IsKnownSizeVector)
+                    if (!type.IsKnownSizeVector())
                         continue;
                     ColumnType typeNames;
                     if ((typeNames = schema[col].Metadata.Schema.GetColumnOrNull(MetadataUtils.Kinds.SlotNames)?.Type) == null)
                         continue;
-                    if (typeNames.VectorSize != type.VectorSize || !(typeNames.ItemType is TextType))
+                    if (typeNames.GetVectorSize() != type.GetVectorSize() || !(typeNames.GetItemType() is TextType))
                     {
                         Contracts.Assert(false, "Unexpected slot names type");
                         continue;
                     }
                     schema[col].Metadata.GetValue(MetadataUtils.Kinds.SlotNames, ref names);
-                    if (names.Length != type.VectorSize)
+                    if (names.Length != type.GetVectorSize())
                     {
                         Contracts.Assert(false, "Unexpected length of slot names vector");
                         continue;
@@ -193,10 +193,10 @@ private static void ShowMetadata(IndentedTextWriter itw, Schema schema, int col,
                     itw.Write("Metadata '{0}': {1}", metaColumn.Name, type);
                     if (showVals)
                     {
-                        if (!type.IsVector)
+                        if (!(type is VectorType vectorType))
                             ShowMetadataValue(itw, schema, col, metaColumn.Name, type);
                         else
-                            ShowMetadataValueVec(itw, schema, col, metaColumn.Name, type);
+                            ShowMetadataValueVec(itw, schema, col, metaColumn.Name, vectorType);
                     }
                     itw.WriteLine();
                 }
@@ -210,7 +210,7 @@ private static void ShowMetadataValue(IndentedTextWriter itw, Schema schema, int
             Contracts.Assert(0 <= col && col < schema.Count);
             Contracts.AssertNonEmpty(kind);
             Contracts.AssertValue(type);
-            Contracts.Assert(!type.IsVector);
+            Contracts.Assert(!(type is VectorType));
 
             if (!type.IsStandardScalar() && !(type is KeyType))
             {
@@ -230,7 +230,7 @@ private static void ShowMetadataValue<T>(IndentedTextWriter itw, Schema schema,
             Contracts.Assert(0 <= col && col < schema.Count);
             Contracts.AssertNonEmpty(kind);
             Contracts.AssertValue(type);
-            Contracts.Assert(!type.IsVector);
+            Contracts.Assert(!(type is VectorType));
             Contracts.Assert(type.RawType == typeof(T));
 
             var conv = Conversions.Instance.GetStringConversion<T>(type);
@@ -243,34 +243,32 @@ private static void ShowMetadataValue<T>(IndentedTextWriter itw, Schema schema,
             itw.Write(": '{0}'", sb);
         }
 
-        private static void ShowMetadataValueVec(IndentedTextWriter itw, Schema schema, int col, string kind, ColumnType type)
+        private static void ShowMetadataValueVec(IndentedTextWriter itw, Schema schema, int col, string kind, VectorType type)
         {
             Contracts.AssertValue(itw);
             Contracts.AssertValue(schema);
             Contracts.Assert(0 <= col && col < schema.Count);
             Contracts.AssertNonEmpty(kind);
             Contracts.AssertValue(type);
-            Contracts.Assert(type.IsVector);
 
             if (!type.ItemType.IsStandardScalar() && !(type.ItemType is KeyType))
             {
                 itw.Write(": Can't display value of this type");
                 return;
             }
 
-            Action<IndentedTextWriter, Schema, int, string, ColumnType> del = ShowMetadataValueVec<int>;
+            Action<IndentedTextWriter, Schema, int, string, VectorType> del = ShowMetadataValueVec<int>;
             var meth = del.GetMethodInfo().GetGenericMethodDefinition().MakeGenericMethod(type.ItemType.RawType);
             meth.Invoke(null, new object[] { itw, schema, col, kind, type });
         }
 
-        private static void ShowMetadataValueVec<T>(IndentedTextWriter itw, Schema schema, int col, string kind, ColumnType type)
+        private static void ShowMetadataValueVec<T>(IndentedTextWriter itw, Schema schema, int col, string kind, VectorType type)
         {
             Contracts.AssertValue(itw);
             Contracts.AssertValue(schema);
             Contracts.Assert(0 <= col && col < schema.Count);
             Contracts.AssertNonEmpty(kind);
             Contracts.AssertValue(type);
-            Contracts.Assert(type.IsVector);
             Contracts.Assert(type.ItemType.RawType == typeof(T));
 
             var conv = Conversions.Instance.GetStringConversion<T>(type.ItemType);

src/Microsoft.ML.Data/Data/Conversion.cs

@@ -671,7 +671,7 @@ private static StringBuilder ClearDst(ref StringBuilder dst)
         public InPredicate<T> GetIsDefaultPredicate<T>(ColumnType type)
         {
             Contracts.CheckValue(type, nameof(type));
-            Contracts.CheckParam(!type.IsVector, nameof(type));
+            Contracts.CheckParam(!(type is VectorType), nameof(type));
             Contracts.CheckParam(type.RawType == typeof(T), nameof(type));
 
             var t = type;
@@ -710,7 +710,7 @@ public bool TryGetIsNAPredicate<T>(ColumnType type, out InPredicate<T> pred)
         public bool TryGetIsNAPredicate(ColumnType type, out Delegate del)
         {
             Contracts.CheckValue(type, nameof(type));
-            Contracts.CheckParam(!type.IsVector, nameof(type));
+            Contracts.CheckParam(!(type is VectorType), nameof(type));
 
             var t = type;
             if (t is KeyType)