changing the signature for IsColumnActive to take a DataViewSchema.Column instead of the column index.

Author: sfilipi

src/Microsoft.Data.DataView/IDataView.cs

@@ -132,9 +132,9 @@ public abstract class DataViewRow : IDisposable
         public abstract ValueGetter<DataViewRowId> GetIdGetter();
 
         /// <summary>
-        /// Returns whether the column with the given index, is active in this row.
+        /// Returns whether the give column is active in this row.
         /// </summary>
-        public abstract bool IsColumnActive(int columnIndex);
+        public abstract bool IsColumnActive(DataViewSchema.Column column);
 
         /// <summary>
         /// Returns a value getter delegate to fetch the value of the given <paramref name="column"/>, from the row.

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

@@ -475,7 +475,7 @@ public AnnotationRow(DataViewSchema.Annotations annotations)
             /// <summary>
             /// Returns whether the given column is active in this row.
             /// </summary>
-            public override bool IsColumnActive(int columnIndex) => true;
+            public override bool IsColumnActive(DataViewSchema.Column column) => true;
         }
 
         /// <summary>

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

@@ -32,10 +32,10 @@ protected LinkedRowRootCursorBase(IChannelProvider provider, DataViewRowCursor i
         /// <summary>
         /// Returns whether the given column is active in this row.
         /// </summary>
-        public sealed override bool IsColumnActive(int columnIndex)
+        public sealed override bool IsColumnActive(DataViewSchema.Column column)
         {
-            Ch.Check(0 <= columnIndex && columnIndex < Schema.Count);
-            return _active == null || _active[columnIndex];
+            Ch.Check(column.Index < Schema.Count);
+            return _active == null || _active[column.Index];
         }
 
         /// <summary>

src/Microsoft.ML.Core/Utilities/Utils.cs

@@ -776,6 +776,42 @@ public static T[] BuildArray<T>(int length, Func<int, T> func)
             return result;
         }
 
+        /// <summary>
+        /// Given a predicate, over a range of values defined by a limit calculate
+        /// first the values for which that predicate was true, and second an inverse
+        /// map.
+        /// </summary>
+        /// <param name="lim">Indicates the exclusive upper bound on the tested values</param>
+        /// <param name="schema">The input schema where the predicate can checkif columns are active.</param>
+        /// <param name="pred">The predicate to test for various value</param>
+        /// <param name="map">An ascending array of values from 0 inclusive
+        /// to <paramref name="lim"/> exclusive, holding all values for which
+        /// <paramref name="pred"/> is true</param>
+        /// <param name="invMap">Forms an inverse mapping of <paramref name="map"/>,
+        /// so that <c><paramref name="invMap"/>[<paramref name="map"/>[i]] == i</c>,
+        /// and for other entries not appearing in <paramref name="map"/>,
+        /// <c><paramref name="invMap"/>[i] == -1</c></param>
+        public static void BuildSubsetMaps(int lim, DataViewSchema schema, Func<DataViewSchema.Column, bool> pred, out int[] map, out int[] invMap)
+        {
+            Contracts.CheckParam(lim >= 0, nameof(lim));
+            Contracts.CheckValue(pred, nameof(pred));
+            // REVIEW: Better names?
+            List<int> mapList = new List<int>();
+            invMap = new int[lim];
+            for (int c = 0; c < lim; ++c)
+            {
+                if (!pred(schema[c]))
+                {
+                    invMap[c] = -1;
+                    continue;
+                }
+                invMap[c] = mapList.Count;
+                mapList.Add(c);
+            }
+            map = mapList.ToArray();
+        }
+
+        //SENJA: REMOVE BEFORE COMMIT
         /// <summary>
         /// Given a predicate, over a range of values defined by a limit calculate
         /// first the values for which that predicate was true, and second an inverse

src/Microsoft.ML.Data/Data/DataViewUtils.cs

@@ -173,14 +173,14 @@ public static DataViewRowCursor[] CreateSplitCursors(IChannelProvider provider,
         /// Return whether all the active columns, as determined by the predicate, are
         /// cachable - either primitive types or vector types.
         /// </summary>
-        public static bool AllCacheable(DataViewSchema schema, Func<int, bool> predicate)
+        public static bool AllCacheable(DataViewSchema schema, Func<DataViewSchema.Column, bool> predicate)
         {
             Contracts.CheckValue(schema, nameof(schema));
             Contracts.CheckValue(predicate, nameof(predicate));
 
             for (int col = 0; col < schema.Count; col++)
             {
-                if (!predicate(col))
+                if (!predicate(schema[col]))
                     continue;
                 var type = schema[col].Type;
                 if (!IsCacheable(type))
@@ -239,10 +239,10 @@ public static bool SameSchemaAndActivity(DataViewRowCursor[] cursors)
             // All cursors must have the same columns active.
             for (int c = 0; c < schema.Count; ++c)
             {
-                bool active = firstCursor.IsColumnActive(c);
+                bool active = firstCursor.IsColumnActive(schema[c]);
                 for (int i = 1; i < cursors.Length; ++i)
                 {
-                    if (cursors[i].IsColumnActive(c) != active)
+                    if (cursors[i].IsColumnActive(schema[c]) != active)
                         return false;
                 }
             }
@@ -334,7 +334,7 @@ private static DataViewRowCursor ConsolidateCore(IChannelProvider provider, Data
 
                 int[] activeToCol;
                 int[] colToActive;
-                Utils.BuildSubsetMaps(schema.Count, cursor.IsColumnActive, out activeToCol, out colToActive);
+                Utils.BuildSubsetMaps(schema.Count, schema, cursor.IsColumnActive, out activeToCol, out colToActive);
 
                 // Because the schema of the consolidator is not necessary fixed, we are merely
                 // opportunistic about buffer sharing, from cursoring to cursoring. If we can do
@@ -517,7 +517,7 @@ private DataViewRowCursor[] SplitCore(IChannelProvider ch, DataViewRowCursor inp
                 // Create the mappings between active column index, and column index.
                 int[] activeToCol;
                 int[] colToActive;
-                Utils.BuildSubsetMaps(_schema.Count, input.IsColumnActive, out activeToCol, out colToActive);
+                Utils.BuildSubsetMaps(_schema.Count, _schema, input.IsColumnActive, out activeToCol, out colToActive);
 
                 Func<DataViewRowCursor, int, InPipe> createFunc = CreateInPipe<int>;
                 var inGenMethod = createFunc.GetMethodInfo().GetGenericMethodDefinition();
@@ -534,14 +534,14 @@ private DataViewRowCursor[] SplitCore(IChannelProvider ch, DataViewRowCursor inp
                 {
                     ch.Assert(0 <= activeToCol[c] && activeToCol[c] < _schema.Count);
                     ch.Assert(c == 0 || activeToCol[c - 1] < activeToCol[c]);
-                    ch.Assert(input.IsColumnActive(activeToCol[c]));
-                    var type = input.Schema[activeToCol[c]].Type;
-                    ch.Assert(type.IsCacheable());
+                    var column = input.Schema[activeToCol[c]];
+                    ch.Assert(input.IsColumnActive(column));
+                    ch.Assert(column.Type.IsCacheable());
                     arguments[1] = activeToCol[c];
                     var inPipe = inPipes[c] =
-                        (InPipe)inGenMethod.MakeGenericMethod(type.RawType).Invoke(this, arguments);
+                        (InPipe)inGenMethod.MakeGenericMethod(column.Type.RawType).Invoke(this, arguments);
                     for (int i = 0; i < cthd; ++i)
-                        outPipes[i][c] = inPipe.CreateOutPipe(type);
+                        outPipes[i][c] = inPipe.CreateOutPipe(column.Type);
                 }
                 // Beyond the InPipes corresponding to column values, we have extra side info pipes.
                 int idIdx = activeToCol.Length + (int)ExtraIndex.Id;
@@ -1105,10 +1105,10 @@ protected override bool MoveNextCore()
                 /// <summary>
                 /// Returns whether the given column is active in this row.
                 /// </summary>
-                public override bool IsColumnActive(int columnIndex)
+                public override bool IsColumnActive(DataViewSchema.Column column)
                 {
-                    Ch.CheckParam(0 <= columnIndex && columnIndex < _colToActive.Length, nameof(columnIndex));
-                    return _colToActive[columnIndex] >= 0;
+                    Ch.CheckParam(column.Index < _colToActive.Length, nameof(column));
+                    return _colToActive[column.Index] >= 0;
                 }
 
                 /// <summary>
@@ -1120,7 +1120,7 @@ public override bool IsColumnActive(int columnIndex)
                 /// <param name="column"> is the output column whose getter should be returned.</param>
                 public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
                 {
-                    Ch.CheckParam(IsColumnActive(column.Index), nameof(column), "requested column not active.");
+                    Ch.CheckParam(IsColumnActive(column), nameof(column), "requested column not active.");
                     Ch.CheckParam(column.Index < _colToActive.Length, nameof(column), "requested column is not active or valid for the Schema.");
 
                     var getter = _getters[_colToActive[column.Index]] as ValueGetter<TValue>;
@@ -1180,7 +1180,7 @@ public SynchronousConsolidatingCursor(IChannelProvider provider, DataViewRowCurs
                 _cursors = cursors;
                 _schema = _cursors[0].Schema;
 
-                Utils.BuildSubsetMaps(_schema.Count, _cursors[0].IsColumnActive, out _activeToCol, out _colToActive);
+                Utils.BuildSubsetMaps(_schema.Count, _schema, _cursors[0].IsColumnActive, out _activeToCol, out _colToActive);
 
                 Func<int, Delegate> func = CreateGetter<int>;
                 _methInfo = func.GetMethodInfo().GetGenericMethodDefinition();
@@ -1251,7 +1251,7 @@ private Delegate CreateGetter<T>(int col)
                     var cursor = _cursors[i];
                     Ch.AssertValue(cursor);
                     Ch.Assert(col < cursor.Schema.Count);
-                    Ch.Assert(cursor.IsColumnActive(col));
+                    Ch.Assert(cursor.IsColumnActive(Schema[col]));
                     Ch.Assert(type.Equals(cursor.Schema[col].Type));
                     getters[i] = _cursors[i].GetGetter<T>(cursor.Schema[col]);
                 }
@@ -1296,10 +1296,10 @@ protected override bool MoveNextCore()
             /// <summary>
             /// Returns whether the given column is active in this row.
             /// </summary>
-            public override bool IsColumnActive(int columnIndex)
+            public override bool IsColumnActive(DataViewSchema.Column column)
             {
-                Ch.CheckParam(0 <= columnIndex && columnIndex < _colToActive.Length, nameof(columnIndex));
-                return _colToActive[columnIndex] >= 0;
+                Ch.CheckParam(column.Index < _colToActive.Length, nameof(column));
+                return _colToActive[column.Index] >= 0;
             }
 
             /// <summary>
@@ -1311,7 +1311,7 @@ public override bool IsColumnActive(int columnIndex)
             /// <param name="column"> is the output column whose getter should be returned.</param>
             public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
             {
-                Ch.CheckParam(IsColumnActive(column.Index), nameof(column), "requested column not active");
+                Ch.CheckParam(IsColumnActive(column), nameof(column), "requested column not active");
                 Ch.CheckParam(column.Index < _colToActive.Length, nameof(column), "requested column not active or is invalid for the schema. ");
 
                 var getter = _getters[_colToActive[column.Index]] as ValueGetter<TValue>;

src/Microsoft.ML.Data/Data/RowCursorUtils.cs

@@ -28,7 +28,7 @@ public static Delegate GetGetterAsDelegate(DataViewRow row, int col)
         {
             Contracts.CheckValue(row, nameof(row));
             Contracts.CheckParam(0 <= col && col < row.Schema.Count, nameof(col));
-            Contracts.CheckParam(row.IsColumnActive(col), nameof(col), "column was not active");
+            Contracts.CheckParam(row.IsColumnActive(row.Schema[col]), nameof(col), "column was not active");
 
             Func<DataViewRow, int, Delegate> getGetter = GetGetterAsDelegateCore<int>;
             return Utils.MarshalInvoke(getGetter, row.Schema[col].Type.RawType, row, col);
@@ -51,7 +51,7 @@ public static Delegate GetGetterAs(DataViewType typeDst, DataViewRow row, int co
             Contracts.CheckParam(typeDst is PrimitiveDataViewType, nameof(typeDst));
             Contracts.CheckValue(row, nameof(row));
             Contracts.CheckParam(0 <= col && col < row.Schema.Count, nameof(col));
-            Contracts.CheckParam(row.IsColumnActive(col), nameof(col), "column was not active");
+            Contracts.CheckParam(row.IsColumnActive(row.Schema[col]), nameof(col), "column was not active");
 
             var typeSrc = row.Schema[col].Type;
             Contracts.Check(typeSrc is PrimitiveDataViewType, "Source column type must be primitive");
@@ -72,7 +72,7 @@ public static ValueGetter<TDst> GetGetterAs<TDst>(DataViewType typeDst, DataView
             Contracts.CheckParam(typeDst.RawType == typeof(TDst), nameof(typeDst));
             Contracts.CheckValue(row, nameof(row));
             Contracts.CheckParam(0 <= col && col < row.Schema.Count, nameof(col));
-            Contracts.CheckParam(row.IsColumnActive(col), nameof(col), "column was not active");
+            Contracts.CheckParam(row.IsColumnActive(row.Schema[col]), nameof(col), "column was not active");
 
             var typeSrc = row.Schema[col].Type;
             Contracts.Check(typeSrc is PrimitiveDataViewType, "Source column type must be primitive");
@@ -117,7 +117,7 @@ public static ValueGetter<StringBuilder> GetGetterAsStringBuilder(DataViewRow ro
         {
             Contracts.CheckValue(row, nameof(row));
             Contracts.CheckParam(0 <= col && col < row.Schema.Count, nameof(col));
-            Contracts.CheckParam(row.IsColumnActive(col), nameof(col), "column was not active");
+            Contracts.CheckParam(row.IsColumnActive(row.Schema[col]), nameof(col), "column was not active");
 
             var typeSrc = row.Schema[col].Type;
             Contracts.Check(typeSrc is PrimitiveDataViewType, "Source column type must be primitive");
@@ -150,7 +150,7 @@ public static Delegate GetVecGetterAs(PrimitiveDataViewType typeDst, DataViewRow
             Contracts.CheckValue(typeDst, nameof(typeDst));
             Contracts.CheckValue(row, nameof(row));
             Contracts.CheckParam(0 <= col && col < row.Schema.Count, nameof(col));
-            Contracts.CheckParam(row.IsColumnActive(col), nameof(col), "column was not active");
+            Contracts.CheckParam(row.IsColumnActive(row.Schema[col]), nameof(col), "column was not active");
 
             var typeSrc = row.Schema[col].Type as VectorType;
             Contracts.Check(typeSrc != null, "Source column type must be vector");
@@ -170,7 +170,7 @@ public static ValueGetter<VBuffer<TDst>> GetVecGetterAs<TDst>(PrimitiveDataViewT
             Contracts.CheckParam(typeDst.RawType == typeof(TDst), nameof(typeDst));
             Contracts.CheckValue(row, nameof(row));
             Contracts.CheckParam(0 <= col && col < row.Schema.Count, nameof(col));
-            Contracts.CheckParam(row.IsColumnActive(col), nameof(col), "column was not active");
+            Contracts.CheckParam(row.IsColumnActive(row.Schema[col]), nameof(col), "column was not active");
 
             var typeSrc = row.Schema[col].Type as VectorType;
             Contracts.Check(typeSrc != null, "Source column type must be vector");
@@ -469,7 +469,7 @@ public static IDataView RowAsDataView(IHostEnvironment env, DataViewRow row)
         {
             Contracts.CheckValue(env, nameof(env));
             env.CheckValue(row, nameof(row));
-            env.CheckParam(Enumerable.Range(0, row.Schema.Count).All(c => row.IsColumnActive(c)), nameof(row), "Some columns were inactive");
+            env.CheckParam(Enumerable.Range(0, row.Schema.Count).All(c => row.IsColumnActive(row.Schema[c])), nameof(row), "Some columns were inactive");
             return new OneRowDataView(env, row);
         }
 
@@ -512,7 +512,7 @@ public OneRowDataView(IHostEnvironment env, DataViewRow row)
                 Contracts.AssertValue(env);
                 _host = env.Register("OneRowDataView");
                 _host.AssertValue(row);
-                _host.Assert(Enumerable.Range(0, row.Schema.Count).All(c => row.IsColumnActive(c)));
+                _host.Assert(Enumerable.Range(0, row.Schema.Count).All(c => row.IsColumnActive(row.Schema[c])));
 
                 _row = row;
             }
@@ -565,7 +565,7 @@ public Cursor(IHost host, OneRowDataView parent, bool[] active)
                 public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
                 {
                     Ch.CheckParam(column.Index < Schema.Count, nameof(column));
-                    Ch.CheckParam(IsColumnActive(column.Index), nameof(column.Index), "Requested column is not active.");
+                    Ch.CheckParam(IsColumnActive(column), nameof(column.Index), "Requested column is not active.");
 
                     var getter = _parent._row.GetGetter<TValue>(column);
                     return
@@ -579,13 +579,13 @@ public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column colu
                 /// <summary>
                 /// Returns whether the given column is active in this row.
                 /// </summary>
-                public override bool IsColumnActive(int columnIndex)
+                public override bool IsColumnActive(DataViewSchema.Column column)
                 {
-                    Ch.CheckParam(0 <= columnIndex && columnIndex < Schema.Count, nameof(columnIndex));
+                    Ch.CheckParam(column.Index < Schema.Count, nameof(column));
                     // We present the "illusion" that this column is not active, even though it must be
                     // in the input row.
-                    Ch.Assert(_parent._row.IsColumnActive(columnIndex));
-                    return _active[columnIndex];
+                    Ch.Assert(_parent._row.IsColumnActive(column));
+                    return _active[column.Index];
                 }
 
                 public override ValueGetter<DataViewRowId> GetIdGetter()

src/Microsoft.ML.Data/DataLoadSave/Binary/BinaryLoader.cs

@@ -1989,10 +1989,10 @@ public override Delegate GetGetter()
             /// <summary>
             /// Returns whether the given column is active in this row.
             /// </summary>
-            public override bool IsColumnActive(int columnIndex)
+            public override bool IsColumnActive(DataViewSchema.Column column)
             {
-                Ch.CheckParam(0 <= columnIndex && columnIndex < _colToActivesIndex.Length, nameof(columnIndex));
-                return _colToActivesIndex[columnIndex] >= 0;
+                Ch.CheckParam(column.Index < _colToActivesIndex.Length, nameof(column));
+                return _colToActivesIndex[column.Index] >= 0;
             }
 
             protected override bool MoveNextCore()