Skip to content

Commit f5b4664

Browse files
sfilipisfilipi
committed
addressing PR review, and changing the GetGetter method signature, to take a column, rather than column index.
1 parent 784ab37 commit f5b4664

File tree

136 files changed

+960
-958
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

136 files changed

+960
-958
lines changed

‎src/Microsoft.Data.DataView/DataViewSchema.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -217,14 +217,14 @@ private void CheckGetter<TValue>(Delegate getter)
217217
/// <summary>
218218
/// Get a getter delegate for one value of the annotations row.
219219
/// </summary>
220-
public ValueGetter<TValue> GetGetter<TValue>(int columnIndex)
220+
public ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
221221
{
222-
if (!(0 <= columnIndex && columnIndex < Schema.Count))
223-
throw new ArgumentOutOfRangeException(nameof(columnIndex));
224-
var typedGetter = _getters[columnIndex] as ValueGetter<TValue>;
222+
if (column.Index >= _getters.Length)
223+
throw new ArgumentException(nameof(column));
224+
var typedGetter = _getters[column.Index] as ValueGetter<TValue>;
225225
if (typedGetter == null)
226226
{
227-
Debug.Assert(_getters[columnIndex] != null);
227+
Debug.Assert(_getters[column.Index] != null);
228228
throw new InvalidOperationException($"Invalid call to '{nameof(GetGetter)}'");
229229
}
230230
return typedGetter;
@@ -238,7 +238,7 @@ public void GetValue<TValue>(string kind, ref TValue value)
238238
var column = Schema.GetColumnOrNull(kind);
239239
if (column == null)
240240
throw new InvalidOperationException($"Invalid call to '{nameof(GetValue)}'");
241-
GetGetter<TValue>(column.Value.Index)(ref value);
241+
GetGetter<TValue>(column.Value)(ref value);
242242
}
243243

244244
public override string ToString() => string.Join(", ", Schema.Select(x => x.Name));

‎src/Microsoft.Data.DataView/IDataView.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,13 @@ public abstract class DataViewRow : IDisposable
137137
public abstract bool IsColumnActive(int columnIndex);
138138

139139
/// <summary>
140-
/// Returns a value getter delegate to fetch the valueof column with the given columnIndex, from the row.
140+
/// Returns a value getter delegate to fetch the value of the given <paramref name="column"/>, from the row.
141141
/// This throws if the column is not active in this row, or if the type
142142
/// <typeparamref name="TValue"/> differs from this column's type.
143143
/// </summary>
144144
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
145-
/// <param name="columnIndex"> is the index of a output column whose getter should be returned.</param>
146-
public abstract ValueGetter<TValue> GetGetter<TValue>(int columnIndex);
145+
/// <param name="column"> is the output column whose getter should be returned.</param>
146+
public abstract ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column);
147147

148148
/// <summary>
149149
/// Gets a <see cref="Schema"/>, which provides name and type information for variables

‎src/Microsoft.Data.DataView/SchemaDebuggerProxy.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ private static List<KeyValuePair<string, object>> BuildValues(DataViewSchema.Ann
5050
private static object GetValue<T>(DataViewSchema.Annotations annotations, int columnIndex)
5151
{
5252
T value = default;
53-
annotations.GetGetter<T>(columnIndex)(ref value);
53+
annotations.GetGetter<T>(annotations.Schema[columnIndex])(ref value);
5454
return value;
5555
}
5656
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -462,13 +462,13 @@ public AnnotationRow(DataViewSchema.Annotations annotations)
462462
public override long Batch => 0;
463463

464464
/// <summary>
465-
/// Returns a value getter delegate to fetch the valueof column with the given columnIndex, from the row.
465+
/// Returns a value getter delegate to fetch the value of column with the given columnIndex, from the row.
466466
/// This throws if the column is not active in this row, or if the type
467467
/// <typeparamref name="TValue"/> differs from this column's type.
468468
/// </summary>
469469
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
470-
/// <param name="columnIndex"> is the index of a output column whose getter should be returned.</param>
471-
public override ValueGetter<TValue> GetGetter<TValue>(int columnIndex) => _annotations.GetGetter<TValue>(columnIndex);
470+
/// <param name="column"> is the output column whose getter should be returned.</param>
471+
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column) => _annotations.GetGetter<TValue>(column);
472472

473473
public override ValueGetter<DataViewRowId> GetIdGetter() => (ref DataViewRowId dst) => dst = default;
474474

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace Microsoft.ML.Data
99
/// <summary>
1010
/// A base class for a <see cref="DataViewRowCursor"/> that has an input cursor, but still needs to do work on
1111
/// <see cref="DataViewRowCursor.MoveNext"/>. Note that the default
12-
/// <see cref="LinkedRowRootCursorBase.GetGetter{TValue}(int)"/> assumes that each input column is exposed as an
12+
/// <see cref="LinkedRowRootCursorBase.GetGetter{TValue}(DataViewSchema.Column)"/> assumes that each input column is exposed as an
1313
/// output column with the same column index.
1414
/// </summary>
1515
[BestFriend]
@@ -39,15 +39,15 @@ public sealed override bool IsColumnActive(int columnIndex)
3939
}
4040

4141
/// <summary>
42-
/// Returns a value getter delegate to fetch the valueof column with the given columnIndex, from the row.
42+
/// Returns a value getter delegate to fetch the value of column with the given columnIndex, from the row.
4343
/// This throws if the column is not active in this row, or if the type
4444
/// <typeparamref name="TValue"/> differs from this column's type.
4545
/// </summary>
4646
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
47-
/// <param name="columnIndex"> is the index of a output column whose getter should be returned.</param>
48-
public override ValueGetter<TValue> GetGetter<TValue>(int columnIndex)
47+
/// <param name="column"> is the output column whose getter should be returned.</param>
48+
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
4949
{
50-
return Input.GetGetter<TValue>(columnIndex);
50+
return Input.GetGetter<TValue>(column);
5151
}
5252
}
5353
}

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

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,7 @@ private InPipe CreateInPipe<T>(DataViewRow input, int col)
641641
{
642642
Contracts.AssertValue(input);
643643
Contracts.Assert(0 <= col && col < _schema.Count);
644-
return CreateInPipeCore(col, input.GetGetter<T>(col));
644+
return CreateInPipeCore(col, input.GetGetter<T>(_schema[col]));
645645
}
646646

647647
/// <summary>
@@ -842,7 +842,7 @@ public void SetAll(OutPipe[] pipes)
842842

843843
/// <summary>
844844
/// This helps a cursor present the results of a <see cref="BatchColumn"/>. Practically its role
845-
/// really is to just provide a stable delegate for the <see cref="DataViewRow.GetGetter{T}(int)"/>.
845+
/// really is to just provide a stable delegate for the <see cref="DataViewRow.GetGetter{T}(DataViewSchema.Column)"/>.
846846
/// There is one of these created per column, per output cursor, i.e., in splitting
847847
/// there are <c>n</c> of these created per column, and when consolidating only one of these
848848
/// is created per column.
@@ -1112,16 +1112,18 @@ public override bool IsColumnActive(int columnIndex)
11121112
}
11131113

11141114
/// <summary>
1115-
/// Returns a value getter delegate to fetch the valueof column with the given columnIndex, from the row.
1115+
/// Returns a value getter delegate to fetch the value of column with the given columnIndex, from the row.
11161116
/// This throws if the column is not active in this row, or if the type
11171117
/// <typeparamref name="TValue"/> differs from this column's type.
11181118
/// </summary>
11191119
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
1120-
/// <param name="columnIndex"> is the index of a output column whose getter should be returned.</param>
1121-
public override ValueGetter<TValue> GetGetter<TValue>(int columnIndex)
1120+
/// <param name="column"> is the output column whose getter should be returned.</param>
1121+
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
11221122
{
1123-
Ch.CheckParam(IsColumnActive(columnIndex), nameof(columnIndex), "requested column not active");
1124-
var getter = _getters[_colToActive[columnIndex]] as ValueGetter<TValue>;
1123+
Ch.CheckParam(IsColumnActive(column.Index), nameof(column), "requested column not active.");
1124+
Ch.CheckParam(column.Index < _colToActive.Length, nameof(column), "requested column is not active or valid for the Schema.");
1125+
1126+
var getter = _getters[_colToActive[column.Index]] as ValueGetter<TValue>;
11251127
if (getter == null)
11261128
throw Ch.Except("Invalid TValue: '{0}'", typeof(TValue));
11271129
return getter;
@@ -1251,7 +1253,7 @@ private Delegate CreateGetter<T>(int col)
12511253
Ch.Assert(col < cursor.Schema.Count);
12521254
Ch.Assert(cursor.IsColumnActive(col));
12531255
Ch.Assert(type.Equals(cursor.Schema[col].Type));
1254-
getters[i] = _cursors[i].GetGetter<T>(col);
1256+
getters[i] = _cursors[i].GetGetter<T>(cursor.Schema[col]);
12551257
}
12561258
ValueGetter<T> mine =
12571259
(ref T value) =>
@@ -1301,16 +1303,18 @@ public override bool IsColumnActive(int columnIndex)
13011303
}
13021304

13031305
/// <summary>
1304-
/// Returns a value getter delegate to fetch the valueof column with the given columnIndex, from the row.
1306+
/// Returns a value getter delegate to fetch the value of column with the given columnIndex, from the row.
13051307
/// This throws if the column is not active in this row, or if the type
13061308
/// <typeparamref name="TValue"/> differs from this column's type.
13071309
/// </summary>
13081310
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
1309-
/// <param name="columnIndex"> is the index of a output column whose getter should be returned.</param>
1310-
public override ValueGetter<TValue> GetGetter<TValue>(int columnIndex)
1311+
/// <param name="column"> is the output column whose getter should be returned.</param>
1312+
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
13111313
{
1312-
Ch.CheckParam(IsColumnActive(columnIndex), nameof(columnIndex), "requested column not active");
1313-
var getter = _getters[_colToActive[columnIndex]] as ValueGetter<TValue>;
1314+
Ch.CheckParam(IsColumnActive(column.Index), nameof(column), "requested column not active");
1315+
Ch.CheckParam(column.Index < _colToActive.Length, nameof(column), "requested column not active or is invalid for the schema. ");
1316+
1317+
var getter = _getters[_colToActive[column.Index]] as ValueGetter<TValue>;
13141318
if (getter == null)
13151319
throw Ch.Except("Invalid TValue: '{0}'", typeof(TValue));
13161320
return getter;
@@ -1347,7 +1351,7 @@ public static ValueGetter<ReadOnlyMemory<char>>[] PopulateGetterArray(DataViewRo
13471351

13481352
public static ValueGetter<ReadOnlyMemory<char>> GetSingleValueGetter<T>(DataViewRow cursor, int i, DataViewType colType)
13491353
{
1350-
var floatGetter = cursor.GetGetter<T>(i);
1354+
var floatGetter = cursor.GetGetter<T>(cursor.Schema[i]);
13511355
T v = default(T);
13521356
ValueMapper<T, StringBuilder> conversion;
13531357
if (!Conversions.Instance.TryGetStringConversion<T>(colType, out conversion))
@@ -1377,7 +1381,7 @@ public static ValueGetter<ReadOnlyMemory<char>> GetSingleValueGetter<T>(DataView
13771381

13781382
public static ValueGetter<ReadOnlyMemory<char>> GetVectorFlatteningGetter<T>(DataViewRow cursor, int colIndex, DataViewType colType)
13791383
{
1380-
var vecGetter = cursor.GetGetter<VBuffer<T>>(colIndex);
1384+
var vecGetter = cursor.GetGetter<VBuffer<T>>(cursor.Schema[colIndex]);
13811385
var vbuf = default(VBuffer<T>);
13821386
const int previewValues = 100;
13831387
ValueMapper<T, StringBuilder> conversion;

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

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public static Delegate GetGetterAsDelegate(DataViewRow row, int col)
3636

3737
private static Delegate GetGetterAsDelegateCore<TValue>(DataViewRow row, int col)
3838
{
39-
return row.GetGetter<TValue>(col);
39+
return row.GetGetter<TValue>(row.Schema[col]);
4040
}
4141

4242
/// <summary>
@@ -87,7 +87,7 @@ private static ValueGetter<TDst> GetGetterAsCore<TSrc, TDst>(DataViewType typeSr
8787
Contracts.Assert(typeof(TSrc) == typeSrc.RawType);
8888
Contracts.Assert(typeof(TDst) == typeDst.RawType);
8989

90-
var getter = row.GetGetter<TSrc>(col);
90+
var getter = row.GetGetter<TSrc>(row.Schema[col]);
9191
bool identity;
9292
var conv = Conversions.Instance.GetStandardConversion<TSrc, TDst>(typeSrc, typeDst, out identity);
9393
if (identity)
@@ -128,7 +128,7 @@ private static ValueGetter<StringBuilder> GetGetterAsStringBuilderCore<TSrc>(Dat
128128
{
129129
Contracts.Assert(typeof(TSrc) == typeSrc.RawType);
130130

131-
var getter = row.GetGetter<TSrc>(col);
131+
var getter = row.GetGetter<TSrc>(row.Schema[col]);
132132
var conv = Conversions.Instance.GetStringConversion<TSrc>(typeSrc);
133133

134134
var src = default(TSrc);
@@ -227,7 +227,7 @@ public RowImpl(DataViewRow row, int col)
227227

228228
public override ValueGetter<TValue> GetGetter<TValue>()
229229
{
230-
return _row.GetGetter<TValue>(_col);
230+
return _row.GetGetter<TValue>(_row.Schema[_col]);
231231
}
232232
}
233233

@@ -307,7 +307,7 @@ public static Func<bool> GetIsNewGroupDelegate(DataViewRow cursor, int col)
307307

308308
private static Func<bool> GetIsNewGroupDelegateCore<T>(DataViewRow cursor, int col)
309309
{
310-
var getter = cursor.GetGetter<T>(col);
310+
var getter = cursor.GetGetter<T>(cursor.Schema[col]);
311311
bool first = true;
312312
T old = default(T);
313313
T val = default(T);
@@ -349,11 +349,11 @@ public static ValueGetter<Single> GetLabelGetter(DataViewRow cursor, int labelIn
349349
var type = cursor.Schema[labelIndex].Type;
350350

351351
if (type == NumberDataViewType.Single)
352-
return cursor.GetGetter<Single>(labelIndex);
352+
return cursor.GetGetter<Single>(cursor.Schema[labelIndex]);
353353

354354
if (type == NumberDataViewType.Double)
355355
{
356-
var getSingleSrc = cursor.GetGetter<Double>(labelIndex);
356+
var getSingleSrc = cursor.GetGetter<Double>(cursor.Schema[labelIndex]);
357357
return
358358
(ref Single dst) =>
359359
{
@@ -375,7 +375,7 @@ private static ValueGetter<Single> GetLabelGetterNotFloat(DataViewRow cursor, in
375375
// boolean type label mapping: True -> 1, False -> 0.
376376
if (type is BooleanDataViewType)
377377
{
378-
var getBoolSrc = cursor.GetGetter<bool>(labelIndex);
378+
var getBoolSrc = cursor.GetGetter<bool>(cursor.Schema[labelIndex]);
379379
return
380380
(ref Single dst) =>
381381
{
@@ -450,7 +450,7 @@ public static T Fetch<T>(IExceptionContext ectx, DataViewRow row, string name)
450450
if (!row.Schema.TryGetColumnIndex(name, out int col))
451451
throw ectx.Except($"Could not find column '{name}'");
452452
T val = default;
453-
row.GetGetter<T>(col)(ref val);
453+
row.GetGetter<T>(row.Schema[col])(ref val);
454454
return val;
455455
}
456456

@@ -556,17 +556,18 @@ public Cursor(IHost host, OneRowDataView parent, bool[] active)
556556
protected override bool MoveNextCore() => Position < 0;
557557

558558
/// <summary>
559-
/// Returns a value getter delegate to fetch the valueof column with the given columnIndex, from the row.
559+
/// Returns a value getter delegate to fetch the value of column with the given columnIndex, from the row.
560560
/// This throws if the column is not active in this row, or if the type
561561
/// <typeparamref name="TValue"/> differs from this column's type.
562562
/// </summary>
563563
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
564-
/// <param name="columnIndex"> is the index of a output column whose getter should be returned.</param>
565-
public override ValueGetter<TValue> GetGetter<TValue>(int columnIndex)
564+
/// <param name="column"> is the output column whose getter should be returned.</param>
565+
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
566566
{
567-
Ch.CheckParam(0 <= columnIndex && columnIndex < Schema.Count, nameof(columnIndex));
568-
Ch.CheckParam(IsColumnActive(columnIndex), nameof(columnIndex), "Requested column is not active");
569-
var getter = _parent._row.GetGetter<TValue>(columnIndex);
567+
Ch.CheckParam(column.Index < Schema.Count, nameof(column));
568+
Ch.CheckParam(IsColumnActive(column.Index), nameof(column.Index), "Requested column is not active.");
569+
570+
var getter = _parent._row.GetGetter<TValue>(column);
570571
return
571572
(ref TValue val) =>
572573
{
@@ -601,7 +602,7 @@ public override ValueGetter<DataViewRowId> GetIdGetter()
601602

602603
/// <summary>
603604
/// This is an error message meant to be used in the situation where a user calls a delegate as returned from
604-
/// <see cref="DataViewRow.GetIdGetter"/> or <see cref="DataViewRow.GetGetter{TValue}(int)"/>.
605+
/// <see cref="DataViewRow.GetIdGetter"/> or <see cref="DataViewRow.GetGetter{TValue}(DataViewSchema.Column)"/>.
605606
/// </summary>
606607
[BestFriend]
607608
internal const string FetchValueStateError = "Values cannot be fetched at this time. This method was called either before the first call to "

‎src/Microsoft.ML.Data/DataDebuggerPreview.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,13 @@ public override string ToString()
6464

6565
private Action<RowInfo, List<object>> MakeSetter<T>(DataViewRow row, int col)
6666
{
67-
var getter = row.GetGetter<T>(col);
68-
string name = row.Schema[col].Name;
67+
var column = row.Schema[col];
68+
var getter = row.GetGetter<T>(column);
6969
Action<RowInfo, List<object>> result = (rowInfo, list) =>
7070
{
7171
T value = default;
7272
getter(ref value);
73-
rowInfo.Values[col] = new KeyValuePair<string, object>(name, value);
73+
rowInfo.Values[col] = new KeyValuePair<string, object>(column.Name, value);
7474

7575
// Call getter again on another buffer, since we store it in two places.
7676
value = default;

0 commit comments

Comments
 (0)