From df191cbee86f1b5f86a73b066ffba0dd23d47f18 Mon Sep 17 00:00:00 2001 From: Yael Dekel Date: Fri, 22 Mar 2019 14:46:43 -0700 Subject: [PATCH 1/3] Fix bug in text loader when a field/property of the type loaded is missing LoadColumnAttribute --- .../DataLoadSave/Text/TextLoader.cs | 3 ++- test/Microsoft.ML.Tests/TextLoaderTests.cs | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.ML.Data/DataLoadSave/Text/TextLoader.cs b/src/Microsoft.ML.Data/DataLoadSave/Text/TextLoader.cs index c4eaf27d68..6ead7a1578 100644 --- a/src/Microsoft.ML.Data/DataLoadSave/Text/TextLoader.cs +++ b/src/Microsoft.ML.Data/DataLoadSave/Text/TextLoader.cs @@ -1460,7 +1460,8 @@ internal static TextLoader CreateTextLoader(IHostEnvironment host, var memberInfo = memberInfos[index]; var mappingAttr = memberInfo.GetCustomAttribute(); - host.Assert(mappingAttr != null, $"Field or property {memberInfo.Name} is missing the {nameof(LoadColumnAttribute)} attribute"); + if (mappingAttr == null) + throw host.Except($"{(memberInfo is FieldInfo ? "Field" : "Property")} '{memberInfo.Name}' is missing the {nameof(LoadColumnAttribute)} attribute"); var mappingAttrName = memberInfo.GetCustomAttribute(); diff --git a/test/Microsoft.ML.Tests/TextLoaderTests.cs b/test/Microsoft.ML.Tests/TextLoaderTests.cs index 06f455d8f7..6dbc7f86d5 100644 --- a/test/Microsoft.ML.Tests/TextLoaderTests.cs +++ b/test/Microsoft.ML.Tests/TextLoaderTests.cs @@ -598,6 +598,29 @@ public void ThrowsExceptionWithPropertyName() catch (NullReferenceException) { }; } + public class DataWithNoAttribute + { + [LoadColumn(0)] + public float Label { get; set; } + public float Feature { get; set; } + } + + [Fact] + public void ThrowsExceptionWithNoColumnAttribute() + { + var mlContext = new MLContext(seed: 1); + try + { + var data = mlContext.Data.LoadFromTextFile("path.txt"); + } + catch (InvalidOperationException e) + { + Assert.Contains($"Property 'Feature' is missing the {nameof(LoadColumnAttribute)} attribute", e.Message); + } + + + } + [Fact] public void ParseSchemaFromTextFile() { From 5d730cd5c81b76751216d9c10d75481fa2d1a753 Mon Sep 17 00:00:00 2001 From: Yael Dekel Date: Fri, 22 Mar 2019 14:50:01 -0700 Subject: [PATCH 2/3] Remove whitespace --- test/Microsoft.ML.Tests/TextLoaderTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/Microsoft.ML.Tests/TextLoaderTests.cs b/test/Microsoft.ML.Tests/TextLoaderTests.cs index 6dbc7f86d5..1da1f11b90 100644 --- a/test/Microsoft.ML.Tests/TextLoaderTests.cs +++ b/test/Microsoft.ML.Tests/TextLoaderTests.cs @@ -617,8 +617,6 @@ public void ThrowsExceptionWithNoColumnAttribute() { Assert.Contains($"Property 'Feature' is missing the {nameof(LoadColumnAttribute)} attribute", e.Message); } - - } [Fact] From 93a3213fe0eb3af335a7df96bc42e0739c9a046f Mon Sep 17 00:00:00 2001 From: Yael Dekel Date: Fri, 22 Mar 2019 18:25:41 -0700 Subject: [PATCH 3/3] Use existing unit test instead of new one. --- test/Microsoft.ML.Tests/TextLoaderTests.cs | 30 ++-------------------- 1 file changed, 2 insertions(+), 28 deletions(-) diff --git a/test/Microsoft.ML.Tests/TextLoaderTests.cs b/test/Microsoft.ML.Tests/TextLoaderTests.cs index 1da1f11b90..e22df9aabb 100644 --- a/test/Microsoft.ML.Tests/TextLoaderTests.cs +++ b/test/Microsoft.ML.Tests/TextLoaderTests.cs @@ -589,34 +589,8 @@ public void CanSuccessfullyTrimSpaces() public void ThrowsExceptionWithPropertyName() { var mlContext = new MLContext(seed: 1); - try - { - mlContext.Data.LoadFromTextFile("fakefile.txt"); - } - // REVIEW: the issue of different exceptions being thrown is tracked under #2037. - catch (Xunit.Sdk.TrueException) { } - catch (NullReferenceException) { }; - } - - public class DataWithNoAttribute - { - [LoadColumn(0)] - public float Label { get; set; } - public float Feature { get; set; } - } - - [Fact] - public void ThrowsExceptionWithNoColumnAttribute() - { - var mlContext = new MLContext(seed: 1); - try - { - var data = mlContext.Data.LoadFromTextFile("path.txt"); - } - catch (InvalidOperationException e) - { - Assert.Contains($"Property 'Feature' is missing the {nameof(LoadColumnAttribute)} attribute", e.Message); - } + var ex = Assert.Throws(() => mlContext.Data.LoadFromTextFile("fakefile.txt")); + Assert.StartsWith($"Field 'String1' is missing the {nameof(LoadColumnAttribute)} attribute", ex.Message); } [Fact]