Skip to content

Commit a7cd3d8

Browse files
codemzscodemzs
committed
PR feedback.
1 parent 9a4f192 commit a7cd3d8

File tree

6 files changed

+38
-58
lines changed

6 files changed

+38
-58
lines changed

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

Lines changed: 23 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,7 @@ public void Convert(ref BL src, ref SB dst)
10351035
/// </summary>
10361036
public bool TryParse(ref TX src, out U1 dst)
10371037
{
1038+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to unsigned integer type.");
10381039
ulong res;
10391040
if (!TryParse(ref src, out res) || res > U1.MaxValue)
10401041
{
@@ -1050,6 +1051,7 @@ public bool TryParse(ref TX src, out U1 dst)
10501051
/// </summary>
10511052
public bool TryParse(ref TX src, out U2 dst)
10521053
{
1054+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to unsigned integer type.");
10531055
ulong res;
10541056
if (!TryParse(ref src, out res) || res > U2.MaxValue)
10551057
{
@@ -1065,6 +1067,7 @@ public bool TryParse(ref TX src, out U2 dst)
10651067
/// </summary>
10661068
public bool TryParse(ref TX src, out U4 dst)
10671069
{
1070+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to unsigned integer type.");
10681071
ulong res;
10691072
if (!TryParse(ref src, out res) || res > U4.MaxValue)
10701073
{
@@ -1080,12 +1083,7 @@ public bool TryParse(ref TX src, out U4 dst)
10801083
/// </summary>
10811084
public bool TryParse(ref TX src, out U8 dst)
10821085
{
1083-
if (src.IsNA)
1084-
{
1085-
dst = 0;
1086-
return false;
1087-
}
1088-
1086+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to unsigned integer type.");
10891087
int ichMin;
10901088
int ichLim;
10911089
string text = src.GetRawUnderlyingBufferInfo(out ichMin, out ichLim);
@@ -1203,6 +1201,8 @@ private bool IsStdMissing(ref TX src)
12031201
/// </summary>
12041202
public bool TryParseKey(ref TX src, U8 min, U8 max, out U8 dst)
12051203
{
1204+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to unsigned integer type.");
1205+
Contracts.Check(!IsStdMissing(ref src), "Missing text value cannot be converted to unsigned integer type.");
12061206
Contracts.Assert(min <= max);
12071207

12081208
// This simply ensures we don't have min == 0 and max == U8.MaxValue. This is illegal since
@@ -1227,7 +1227,7 @@ public bool TryParseKey(ref TX src, U8 min, U8 max, out U8 dst)
12271227
{
12281228
dst = 0;
12291229
// Return true only for standard forms for NA.
1230-
return IsStdMissing(ref src);
1230+
return false;
12311231
}
12321232

12331233
if (min > uu || uu > max)
@@ -1277,13 +1277,9 @@ private bool TryParseCore(string text, int ich, int lim, out ulong dst)
12771277
/// </summary>
12781278
public bool TryParse(ref TX src, out I1 dst)
12791279
{
1280-
dst = default;
1281-
if (src.IsNA)
1282-
{
1283-
dst = I1.MinValue;
1284-
return true;
1285-
}
1280+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to integer type.");
12861281

1282+
dst = default;
12871283
TryParseSigned(I1.MaxValue, ref src, out long? res);
12881284
Contracts.Check(res.HasValue, "Value could not be parsed from text to sbyte.");
12891285
Contracts.Check((I1)res == res, "Overflow or underflow occured while converting value in text to sbyte.");
@@ -1297,13 +1293,9 @@ public bool TryParse(ref TX src, out I1 dst)
12971293
/// </summary>
12981294
public bool TryParse(ref TX src, out I2 dst)
12991295
{
1300-
dst = default;
1301-
if (src.IsNA)
1302-
{
1303-
dst = I2.MinValue;
1304-
return true;
1305-
}
1296+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to integer type.");
13061297

1298+
dst = default;
13071299
TryParseSigned(I2.MaxValue, ref src, out long? res);
13081300
Contracts.Check(res.HasValue, "Value could not be parsed from text to short.");
13091301
Contracts.Check((I2)res == res, "Overflow or underflow occured while converting value in text to short.");
@@ -1317,13 +1309,9 @@ public bool TryParse(ref TX src, out I2 dst)
13171309
/// </summary>
13181310
public bool TryParse(ref TX src, out I4 dst)
13191311
{
1320-
dst = default;
1321-
if (src.IsNA)
1322-
{
1323-
dst = I4.MinValue;
1324-
return true;
1325-
}
1312+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to integer type.");
13261313

1314+
dst = default;
13271315
TryParseSigned(I4.MaxValue, ref src, out long? res);
13281316
Contracts.Check(res.HasValue, "Value could not be parsed from text to int32.");
13291317
Contracts.Check((I4)res == res, "Overflow or underflow occured while converting value in text to int.");
@@ -1337,13 +1325,9 @@ public bool TryParse(ref TX src, out I4 dst)
13371325
/// </summary>
13381326
public bool TryParse(ref TX src, out I8 dst)
13391327
{
1340-
dst = default;
1341-
if (src.IsNA)
1342-
{
1343-
dst = I8.MinValue;
1344-
return true;
1345-
}
1328+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to integer type.");
13461329

1330+
dst = default;
13471331
TryParseSigned(I8.MaxValue, ref src, out long? res);
13481332
Contracts.Check(res.HasValue, "Value could not be parsed from text to long.");
13491333
dst = (I8)res;
@@ -1525,9 +1509,7 @@ public bool TryParse(ref TX src, out DZ dst)
15251509
// These throw an exception for unparsable and overflow values.
15261510
private I1 ParseI1(ref TX src)
15271511
{
1528-
if (src.IsNA)
1529-
return I1.MinValue;
1530-
1512+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to integer type.");
15311513
TryParseSigned(I1.MaxValue, ref src, out long? res);
15321514
Contracts.Check(res.HasValue, "Value could not be parsed from text to sbyte.");
15331515
Contracts.Check((I1)res == res, "Overflow or underflow occured while converting value in text to sbyte.");
@@ -1536,9 +1518,7 @@ private I1 ParseI1(ref TX src)
15361518

15371519
private I2 ParseI2(ref TX src)
15381520
{
1539-
if (src.IsNA)
1540-
return I2.MinValue;
1541-
1521+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to integer type.");
15421522
TryParseSigned(I2.MaxValue, ref src, out long? res);
15431523
Contracts.Check(res.HasValue, "Value could not be parsed from text to short.");
15441524
Contracts.Check((I2)res == res, "Overflow or underflow occured while converting value in text to short.");
@@ -1547,9 +1527,7 @@ private I2 ParseI2(ref TX src)
15471527

15481528
private I4 ParseI4(ref TX src)
15491529
{
1550-
if (src.IsNA)
1551-
return I4.MinValue;
1552-
1530+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to integer type.");
15531531
TryParseSigned(I4.MaxValue, ref src, out long? res);
15541532
Contracts.Check(res.HasValue, "Value could not be parsed from text to int.");
15551533
Contracts.Check((I4)res == res, "Overflow or underflow occured while converting value in text to int.");
@@ -1558,9 +1536,7 @@ private I4 ParseI4(ref TX src)
15581536

15591537
private I8 ParseI8(ref TX src)
15601538
{
1561-
if (src.IsNA)
1562-
return I8.MinValue;
1563-
1539+
Contracts.Check(!src.IsNA, "Missing text value cannot be converted to integer type.");
15641540
TryParseSigned(I8.MaxValue, ref src, out long? res);
15651541
Contracts.Check(res.HasValue, "Value could not be parsed from text to long.");
15661542
return res.Value;
@@ -1571,6 +1547,7 @@ private I8 ParseI8(ref TX src)
15711547
// unsigned integer types.
15721548
private U1 ParseU1(ref TX span)
15731549
{
1550+
Contracts.Check(!span.IsNA, "Missing text value cannot be converted to unsigned integer type.");
15741551
ulong res;
15751552
if (!TryParse(ref span, out res))
15761553
return 0;
@@ -1581,6 +1558,7 @@ private U1 ParseU1(ref TX span)
15811558

15821559
private U2 ParseU2(ref TX span)
15831560
{
1561+
Contracts.Check(!span.IsNA, "Missing text value cannot be converted to unsigned integer type.");
15841562
ulong res;
15851563
if (!TryParse(ref span, out res))
15861564
return 0;
@@ -1591,6 +1569,7 @@ private U2 ParseU2(ref TX span)
15911569

15921570
private U4 ParseU4(ref TX span)
15931571
{
1572+
Contracts.Check(!span.IsNA, "Missing text value cannot be converted to unsigned integer type.");
15941573
ulong res;
15951574
if (!TryParse(ref span, out res))
15961575
return 0;
@@ -1601,6 +1580,7 @@ private U4 ParseU4(ref TX span)
16011580

16021581
private U8 ParseU8(ref TX span)
16031582
{
1583+
Contracts.Check(!span.IsNA, "Missing text value cannot be converted to unsigned integer type.");
16041584
ulong res;
16051585
if (!TryParse(ref span, out res))
16061586
return 0;

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -729,7 +729,7 @@ public void GetMetadata<TValue>(string kind, int col, ref TValue value)
729729
/// <summary>
730730
/// Upper inclusive bound of versions this reader can read.
731731
/// </summary>
732-
private const ulong ReaderVersion = MissingTextVersion;
732+
private const ulong ReaderVersion = StandardDataTypesVersion;
733733

734734
/// <summary>
735735
/// The first version that removes DvTypes and uses .NET standard
@@ -1096,10 +1096,10 @@ private unsafe Header InitHeader()
10961096
throw _host.Except("Cannot read version {0} data, earliest that can be handled is {1}",
10971097
Header.VersionToString(header.CompatibleVersion), Header.VersionToString(MetadataVersion));
10981098
}
1099-
if (header.CompatibleVersion > StandardDataTypesVersion)
1099+
if (header.CompatibleVersion > ReaderVersion)
11001100
{
11011101
throw _host.Except("Cannot read version {0} data, latest that can be handled is {1}",
1102-
Header.VersionToString(header.CompatibleVersion), Header.VersionToString(StandardDataTypesVersion));
1102+
Header.VersionToString(header.CompatibleVersion), Header.VersionToString(ReaderVersion));
11031103
}
11041104

11051105
_host.CheckDecode(header.RowCount >= 0, "Row count cannot be negative");

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ public struct Header
3434
//public const ulong WriterVersion = 0x0001000100010002; // Codec changes.
3535
//public const ulong WriterVersion = 0x0001000100010003; // Slot names.
3636
//public const ulong WriterVersion = 0x0001000100010004; // Column metadata.
37-
public const ulong WriterVersion = 0x0001000100010005; // "NA" DvText support.
38-
public const ulong CanBeReadByVersion = 0x0001000100010005;
37+
//public const ulong WriterVersion = 0x0001000100010005; // "NA" DvText support.
38+
public const ulong WriterVersion = 0x0001000100010006; // Replace DvTypes with .NET Standard data types.
39+
public const ulong CanBeReadByVersion = 0x0001000100010006;
3940

4041
internal static string VersionToString(ulong v)
4142
{

‎test/Microsoft.ML.Core.Tests/UnitTests/DataTypes.cs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public DataTypesTest(ITestOutputHelper helper)
1919
private readonly static Conversions _conv = Conversions.Instance;
2020

2121
[Fact]
22-
public void TXToI1()
22+
public void TXToSByte()
2323
{
2424
var mapper = GetMapper<DvText, sbyte>();
2525

@@ -86,15 +86,15 @@ public void TXToI1()
8686
}
8787
catch (Exception ex)
8888
{
89-
Assert.Equal("Missing text value cannot be converted to signed numbers.", ex.Message);
89+
Assert.Equal("Missing text value cannot be converted to integer type.", ex.Message);
9090
error = true;
9191
}
9292

9393
Assert.True(error);
9494
}
9595

9696
[Fact]
97-
public void TXToI2()
97+
public void TXToShort()
9898
{
9999
var mapper = GetMapper<DvText, short>();
100100

@@ -161,15 +161,15 @@ public void TXToI2()
161161
}
162162
catch (Exception ex)
163163
{
164-
Assert.Equal("Missing text value cannot be converted to signed numbers.", ex.Message);
164+
Assert.Equal("Missing text value cannot be converted to integer type.", ex.Message);
165165
error = true;
166166
}
167167

168168
Assert.True(error);
169169
}
170170

171171
[Fact]
172-
public void TXToI4()
172+
public void TXToInt()
173173
{
174174
var mapper = GetMapper<DvText, int>();
175175

@@ -236,15 +236,15 @@ public void TXToI4()
236236
}
237237
catch (Exception ex)
238238
{
239-
Assert.Equal("Missing text value cannot be converted to signed numbers.", ex.Message);
239+
Assert.Equal("Missing text value cannot be converted to integer type.", ex.Message);
240240
error = true;
241241
}
242242

243243
Assert.True(error);
244244
}
245245

246246
[Fact]
247-
public void TXToI8()
247+
public void TXToLong()
248248
{
249249
var mapper = GetMapper<DvText, long>();
250250

@@ -301,7 +301,7 @@ public void TXToI8()
301301
}
302302
catch (Exception ex)
303303
{
304-
Assert.Equal("Missing text value cannot be converted to signed numbers.", ex.Message);
304+
Assert.Equal("Missing text value cannot be converted to integer type.", ex.Message);
305305
error = true;
306306
}
307307

‎test/Microsoft.ML.Core.Tests/UnitTests/TestEntryPoints.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ private IDataView GetBreastCancerDataviewWithTextColumns()
6666
{
6767
new TextLoader.Column("Label", type: null, 0),
6868
new TextLoader.Column("F1", DataKind.Text, 1),
69-
new TextLoader.Column("F2", DataKind.I4, 2),
69+
new TextLoader.Column("F2", DataKind.R4, 2),
7070
new TextLoader.Column("Rest", type: null, new [] { new TextLoader.Range(3, 9) })
7171
}
7272
},

‎test/Microsoft.ML.Tests/TextLoaderTests.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ public void TestTextLoaderInvalidLongMax()
113113
string pathData = DeleteOutputPath("SavePipe", "TextInput.txt");
114114
File.WriteAllLines(pathData, new string[] {
115115
"9223372036854775808"
116-
117116
});
118117

119118
try

0 commit comments

Comments
 (0)