Skip to content

Commit 5489b19

Browse files
sfilipisfilipi
committed
pr review comments
1 parent 40ce785 commit 5489b19

40 files changed

+65
-66
lines changed

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public abstract class DataViewRow : IDisposable
132132
public abstract ValueGetter<DataViewRowId> GetIdGetter();
133133

134134
/// <summary>
135-
/// Returns whether the give column is active in this row.
135+
/// Returns whether the given column is active in this row.
136136
/// </summary>
137137
public abstract bool IsColumnActive(DataViewSchema.Column column);
138138

@@ -141,7 +141,7 @@ public abstract class DataViewRow : IDisposable
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>
144-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
144+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
145145
/// <param name="column"> is the output column whose getter should be returned.</param>
146146
public abstract ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column);
147147

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,16 @@ private static List<KeyValuePair<string, object>> BuildValues(DataViewSchema.Ann
4141
foreach (var column in annotations.Schema)
4242
{
4343
var name = column.Name;
44-
var value = Utils.MarshalInvoke(GetValue<int>, column.Type.RawType, annotations, column.Index);
44+
var value = Utils.MarshalInvoke(GetValue<DataViewSchema.Column>, column.Type.RawType, annotations, column);
4545
result.Add(new KeyValuePair<string, object>(name, value));
4646
}
4747
return result;
4848
}
4949

50-
private static object GetValue<T>(DataViewSchema.Annotations annotations, int columnIndex)
50+
private static object GetValue<T>(DataViewSchema.Annotations annotations, DataViewSchema.Column column)
5151
{
5252
T value = default;
53-
annotations.GetGetter<T>(annotations.Schema[columnIndex])(ref value);
53+
annotations.GetGetter<T>(column)(ref value);
5454
return value;
5555
}
5656
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ public AnnotationRow(DataViewSchema.Annotations annotations)
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>
469-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
469+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
470470
/// <param name="column"> is the output column whose getter should be returned.</param>
471471
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column) => _annotations.GetGetter<TValue>(column);
472472

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public sealed override bool IsColumnActive(DataViewSchema.Column column)
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>
46-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
46+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
4747
/// <param name="column"> is the output column whose getter should be returned.</param>
4848
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
4949
{

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

+7-8
Original file line numberDiff line numberDiff line change
@@ -781,24 +781,24 @@ public static T[] BuildArray<T>(int length, Func<int, T> func)
781781
/// first the values for which that predicate was true, and second an inverse
782782
/// map.
783783
/// </summary>
784-
/// <param name="lim">Indicates the exclusive upper bound on the tested values</param>
785-
/// <param name="schema">The input schema where the predicate can checkif columns are active.</param>
784+
/// <param name="schema">The input schema where the predicate can check if columns are active.</param>
786785
/// <param name="pred">The predicate to test for various value</param>
787786
/// <param name="map">An ascending array of values from 0 inclusive
788-
/// to <paramref name="lim"/> exclusive, holding all values for which
787+
/// to <paramref name="schema.Count"/> exclusive, holding all values for which
789788
/// <paramref name="pred"/> is true</param>
790789
/// <param name="invMap">Forms an inverse mapping of <paramref name="map"/>,
791790
/// so that <c><paramref name="invMap"/>[<paramref name="map"/>[i]] == i</c>,
792791
/// and for other entries not appearing in <paramref name="map"/>,
793792
/// <c><paramref name="invMap"/>[i] == -1</c></param>
794-
public static void BuildSubsetMaps(int lim, DataViewSchema schema, Func<DataViewSchema.Column, bool> pred, out int[] map, out int[] invMap)
793+
public static void BuildSubsetMaps(DataViewSchema schema, Func<DataViewSchema.Column, bool> pred, out int[] map, out int[] invMap)
795794
{
796-
Contracts.CheckParam(lim >= 0, nameof(lim));
795+
Contracts.CheckValue(schema, nameof(schema));
796+
Contracts.Check(schema.Count > 0, nameof(schema));
797797
Contracts.CheckValue(pred, nameof(pred));
798798
// REVIEW: Better names?
799799
List<int> mapList = new List<int>();
800-
invMap = new int[lim];
801-
for (int c = 0; c < lim; ++c)
800+
invMap = new int[schema.Count];
801+
for (int c = 0; c < schema.Count; ++c)
802802
{
803803
if (!pred(schema[c]))
804804
{
@@ -811,7 +811,6 @@ public static void BuildSubsetMaps(int lim, DataViewSchema schema, Func<DataView
811811
map = mapList.ToArray();
812812
}
813813

814-
//SENJA: REMOVE BEFORE COMMIT
815814
/// <summary>
816815
/// Given a predicate, over a range of values defined by a limit calculate
817816
/// first the values for which that predicate was true, and second an inverse

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

+5-5
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ private static DataViewRowCursor ConsolidateCore(IChannelProvider provider, Data
334334

335335
int[] activeToCol;
336336
int[] colToActive;
337-
Utils.BuildSubsetMaps(schema.Count, schema, cursor.IsColumnActive, out activeToCol, out colToActive);
337+
Utils.BuildSubsetMaps(schema, cursor.IsColumnActive, out activeToCol, out colToActive);
338338

339339
// Because the schema of the consolidator is not necessary fixed, we are merely
340340
// opportunistic about buffer sharing, from cursoring to cursoring. If we can do
@@ -517,7 +517,7 @@ private DataViewRowCursor[] SplitCore(IChannelProvider ch, DataViewRowCursor inp
517517
// Create the mappings between active column index, and column index.
518518
int[] activeToCol;
519519
int[] colToActive;
520-
Utils.BuildSubsetMaps(_schema.Count, _schema, input.IsColumnActive, out activeToCol, out colToActive);
520+
Utils.BuildSubsetMaps(_schema, input.IsColumnActive, out activeToCol, out colToActive);
521521

522522
Func<DataViewRowCursor, int, InPipe> createFunc = CreateInPipe<int>;
523523
var inGenMethod = createFunc.GetMethodInfo().GetGenericMethodDefinition();
@@ -1116,7 +1116,7 @@ public override bool IsColumnActive(DataViewSchema.Column column)
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>
1119-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
1119+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
11201120
/// <param name="column"> is the output column whose getter should be returned.</param>
11211121
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
11221122
{
@@ -1180,7 +1180,7 @@ public SynchronousConsolidatingCursor(IChannelProvider provider, DataViewRowCurs
11801180
_cursors = cursors;
11811181
_schema = _cursors[0].Schema;
11821182

1183-
Utils.BuildSubsetMaps(_schema.Count, _schema, _cursors[0].IsColumnActive, out _activeToCol, out _colToActive);
1183+
Utils.BuildSubsetMaps(_schema, _cursors[0].IsColumnActive, out _activeToCol, out _colToActive);
11841184

11851185
Func<int, Delegate> func = CreateGetter<int>;
11861186
_methInfo = func.GetMethodInfo().GetGenericMethodDefinition();
@@ -1307,7 +1307,7 @@ public override bool IsColumnActive(DataViewSchema.Column column)
13071307
/// This throws if the column is not active in this row, or if the type
13081308
/// <typeparamref name="TValue"/> differs from this column's type.
13091309
/// </summary>
1310-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
1310+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
13111311
/// <param name="column"> is the output column whose getter should be returned.</param>
13121312
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
13131313
{

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ public Cursor(IHost host, OneRowDataView parent, bool[] active)
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>
563-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
563+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
564564
/// <param name="column"> is the output column whose getter should be returned.</param>
565565
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
566566
{

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -2055,7 +2055,7 @@ protected override bool MoveNextCore()
20552055
/// This throws if the column is not active in this row, or if the type
20562056
/// <typeparamref name="TValue"/> differs from this column's type.
20572057
/// </summary>
2058-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
2058+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
20592059
/// <param name="column"> is the output column whose getter should be returned.</param>
20602060
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
20612061
{

‎src/Microsoft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ public override bool IsColumnActive(DataViewSchema.Column column)
311311
/// This throws if the column is not active in this row, or if the type
312312
/// <typeparamref name="TValue"/> differs from this column's type.
313313
/// </summary>
314-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
314+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
315315
/// <param name="column"> is the output column whose getter should be returned.</param>
316316
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
317317
{

‎src/Microsoft.ML.Data/DataLoadSave/Transpose/TransposeLoader.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,7 @@ public override bool IsColumnActive(DataViewSchema.Column column)
874874
/// This throws if the column is not active in this row, or if the type
875875
/// <typeparamref name="TValue"/> differs from this column's type.
876876
/// </summary>
877-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
877+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
878878
/// <param name="column"> is the output column whose getter should be returned.</param>
879879
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
880880
{

‎src/Microsoft.ML.Data/DataView/ArrayDataViewBuilder.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ public override bool IsColumnActive(DataViewSchema.Column column)
318318
/// This throws if the column is not active in this row, or if the type
319319
/// <typeparamref name="TValue"/> differs from this column's type.
320320
/// </summary>
321-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
321+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
322322
/// <param name="column"> is the output column whose getter should be returned.</param>
323323
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
324324
{

‎src/Microsoft.ML.Data/DataView/CacheDataView.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ public RowSeeker(RowSeekerCore<TWaiter> toWrap)
514514
/// This throws if the column is not active in this row, or if the type
515515
/// <typeparamref name="TValue"/> differs from this column's type.
516516
/// </summary>
517-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
517+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
518518
/// <param name="column"> is the output column whose getter should be returned.</param>
519519
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column) => _internal.GetGetter<TValue>(column);
520520
public override ValueGetter<DataViewRowId> GetIdGetter() => _internal.GetIdGetter();

‎src/Microsoft.ML.Data/DataView/CompositeRowToRowMapper.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public SubsetActive(DataViewRow row, Func<int, bool> pred)
108108
/// This throws if the column is not active in this row, or if the type
109109
/// <typeparamref name="TValue"/> differs from this column's type.
110110
/// </summary>
111-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
111+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
112112
/// <param name="column"> is the output column whose getter should be returned.</param>
113113
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column) => _row.GetGetter<TValue>(column);
114114
public override ValueGetter<DataViewRowId> GetIdGetter() => _row.GetIdGetter();

‎src/Microsoft.ML.Data/DataView/DataViewConstructionUtils.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ private void CheckColumnInRange(int columnIndex)
348348
/// This throws if the column is not active in this row, or if the type
349349
/// <typeparamref name="TValue"/> differs from this column's type.
350350
/// </summary>
351-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
351+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
352352
/// <param name="column"> is the output column whose getter should be returned.</param>
353353
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
354354
{
@@ -432,7 +432,7 @@ protected override void Dispose(bool disposing)
432432
/// This throws if the column is not active in this row, or if the type
433433
/// <typeparamref name="TValue"/> differs from this column's type.
434434
/// </summary>
435-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
435+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
436436
/// <param name="column"> is the output column whose getter should be returned.</param>
437437
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
438438
=> _toWrap.GetGetter<TValue>(column);

‎src/Microsoft.ML.Data/DataView/EmptyDataView.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public override ValueGetter<DataViewRowId> GetIdGetter()
7474
/// This throws if the column is not active in this row, or if the type
7575
/// <typeparamref name="TValue"/> differs from this column's type.
7676
/// </summary>
77-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
77+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
7878
/// <param name="column"> is the output column whose getter should be returned.</param>
7979
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
8080
{

‎src/Microsoft.ML.Data/DataView/RowToRowMapperTransform.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ protected override void DisposeCore(bool disposing)
324324
/// This throws if the column is not active in this row, or if the type
325325
/// <typeparamref name="TValue"/> differs from this column's type.
326326
/// </summary>
327-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
327+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
328328
/// <param name="column"> is the output column whose getter should be returned.</param>
329329
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
330330
{
@@ -386,7 +386,7 @@ public override bool IsColumnActive(DataViewSchema.Column column)
386386
/// This throws if the column is not active in this row, or if the type
387387
/// <typeparamref name="TValue"/> differs from this column's type.
388388
/// </summary>
389-
/// <typeparam name="TValue"> is the output column's content type.</typeparam>
389+
/// <typeparam name="TValue"> is the column's content type.</typeparam>
390390
/// <param name="column"> is the output column whose getter should be returned.</param>
391391
public override ValueGetter<TValue> GetGetter<TValue>(DataViewSchema.Column column)
392392
{

0 commit comments

Comments
 (0)