Add Functional Tests for Data I/O #2518

rogancarr · 2019-02-12T23:50:45Z

This PR adds functional tests for Data I/O

Reading from IEnumerable
Writing to IEnumerable
Writing to and reading from delimited tex files
- With inferred schema
- With explicit schema
Writing to and reading from Binary files

glebuk · 2019-02-13T00:48:05Z

test/Microsoft.ML.Functional.Tests/Common.cs

@@ -20,5 +20,34 @@ public static void CheckMetrics(RegressionMetrics metrics)
            Assert.True(metrics.L2 >= 0);
            Assert.True(metrics.RSquared <= 1);
        }
+
+        public static void AssertEqual(float[] array1, float[] array2)


AssertEqual [](start = 27, length = 11)

Perhaps Enumerable.SequenceEqual(target1, target2); instead? from Linq
https://stackoverflow.com/questions/3232744/easiest-way-to-compare-arrays-in-c-sharp #Closed

We need to use the assert logic here so that we can capture equality to some level of precision. Default equality will fail after serialization due to floating point drift sometimes.

In reply to: 256208383 [](ancestors = 256208383)

Plus I want to Assert and not check for equality.

In reply to: 256209571 [](ancestors = 256209571,256208383)

glebuk · 2019-02-13T00:51:09Z

test/Microsoft.ML.Functional.Tests/Common.cs

+                Assert.Equal(array1[i], array2[i]);
+        }
+
+        public static void AssertEqual(Schema schema1, Schema schema2)


Schema [](start = 39, length = 6)

Schould Schema implement IEquitable ?
https://stackoverflow.com/questions/8400028/comparing-two-instances-of-a-class #Closed

That would be awesome, but probably not before 1.0.

In reply to: 256209052 [](ancestors = 256209052)

+1 on implementing IEquitable. Should have an issue about it.

In reply to: 256209052 [](ancestors = 256209052)

glebuk · 2019-02-13T00:54:00Z

test/Microsoft.ML.Functional.Tests/Common.cs

+        public static void AssertEqual(Schema schema1, Schema schema2)
+        {
+            Assert.NotNull(schema1);
+            Assert.NotNull(schema2);


technically if they are both null they are equal... #Closed

Technically yes. But internally in our library we consider nulls to not equal each other. :shruggy-emoticon:

In reply to: 256209640 [](ancestors = 256209640)

glebuk · 2019-02-13T00:54:25Z

test/Microsoft.ML.Functional.Tests/DataIO.cs

+namespace Microsoft.ML.Functional.Tests
+{
+    /// <summary>
+    /// Test data input and output formats


formats [](start = 35, length = 7)

but what about the dots? #Closed

glebuk · 2019-02-13T00:58:40Z

test/Microsoft.ML.Functional.Tests/DataIO.cs

+        }
+        #endregion
+
+        #region ToyDataset


#region ToyDataset [](start = 7, length = 19)

not a big fan of regions here... #Closed

glebuk · 2019-02-13T01:01:04Z

test/Microsoft.ML.Functional.Tests/DataIO.cs

+
+        private IEnumerable<ToyDataset> GenerateToyDataset(int numExamples = 5, int seed = 1)
+        {
+            var rng = new Random(seed);


ng = new Random(seed); [](start = 17, length = 22)

we can't get random from context? #Closed

Only from IHostEnvironment, I think. That isn't accessible outside the library.

In reply to: 256211132 [](ancestors = 256211132)

glebuk · 2019-02-13T01:01:28Z

test/Microsoft.ML.Functional.Tests/DataIO.cs

+            }
+        }
+
+        private ReadOnlyMemory<char> GetRandomRomChar(Random rng, int length = 10)


GetRandomRomChar [](start = 37, length = 16)

func name unclr - expand ROM #Closed

Used the Span naming convention from .NET.

In reply to: 256211211 [](ancestors = 256211211)

codecov · 2019-02-13T01:02:57Z

Codecov Report

Merging #2518 into master will increase coverage by 0.1%.
The diff coverage is 99.54%.

@@            Coverage Diff            @@
##           master    #2518     +/-   ##
=========================================
+ Coverage   71.25%   71.35%   +0.1%     
=========================================
  Files         798      798             
  Lines      141252   141452    +200     
  Branches    16112    16125     +13     
=========================================
+ Hits       100643   100935    +292     
+ Misses      36145    36045    -100     
- Partials     4464     4472      +8

Flag	Coverage Δ
#Debug	`71.35% <99.54%> (+0.1%)`	⬆️
#production	`67.66% <ø> (+0.08%)`	⬆️
#test	`85.45% <99.54%> (+0.09%)`	⬆️

codecov · 2019-02-13T01:03:11Z

Codecov Report

Merging #2518 into master will increase coverage by 0.09%.
The diff coverage is 98.64%.

@@            Coverage Diff             @@
##           master    #2518      +/-   ##
==========================================
+ Coverage   71.35%   71.45%   +0.09%     
==========================================
  Files         799      800       +1     
  Lines      141496   141700     +204     
  Branches    16120    16135      +15     
==========================================
+ Hits       100971   101247     +276     
+ Misses      36062    35986      -76     
- Partials     4463     4467       +4

Flag	Coverage Δ
#Debug	`71.45% <98.64%> (+0.09%)`	⬆️
#production	`67.75% <ø> (+0.06%)`	⬆️
#test	`85.49% <98.64%> (+0.09%)`	⬆️

glebuk · 2019-02-13T01:03:34Z

test/Microsoft.ML.Functional.Tests/DataIO.cs

+            return new ReadOnlyMemory<char>(chars);
+        }
+
+        private sealed class ToyDataset


This might prove useful.Can we factor toy dataset out into separate file for other tests to share? #Closed

Refactored and called AllTypes.

In reply to: 256211603 [](ancestors = 256211603)

TomFinley · 2019-02-13T08:14:27Z

No less than 7 merges from master into a feature branch. 😄

rogancarr · 2019-02-13T15:00:02Z

No less than 7 merges from master into a feature branch. 😄

I'm still not sure how that happened. I branched off my master and there they were!

sfilipi · 2019-02-13T18:08:43Z

test/Microsoft.ML.Functional.Tests/DataIO.cs

+        /// Write to and read from a delimited file: Schematized data of any DataKind can be read from a delimited file.
+        /// </summary>
+        /// <remarks>
+        /// Tests the roundtrip hrough a file using schema inference.


hrough [](start = 32, length = 6)

typo #Resolved

sfilipi

glebuk · 2019-02-13T18:59:05Z

test/Microsoft.ML.Functional.Tests/Datasets/AllTypes.cs

+
+namespace Microsoft.ML.Functional.Tests.Datasets
+{
+    internal sealed class AllTypes


AllTypes [](start = 26, length = 8)

This name is overly broad. Perhaps a TypeTestData? #Closed

glebuk · 2019-02-13T18:59:50Z

test/Microsoft.ML.Functional.Tests/Datasets/AllTypes.cs

+
+namespace Microsoft.ML.Functional.Tests.Datasets
+{
+    internal sealed class AllTypes


AllTypes [](start = 26, length = 8)

At least one xml summary comment on what it is and what it is for. #Closed

glebuk · 2019-02-13T19:02:51Z

test/Microsoft.ML.Functional.Tests/Datasets/AllTypes.cs

+                    Dt = DateTime.FromOADate(rng.Next(657435, 2958465)),
+                    Dz = DateTimeOffset.FromUnixTimeSeconds((long)(rng.NextDouble() * (1 + rng.Next()))),
+                    Ug = new RowId((ulong)rng.Next(), (ulong)rng.Next())
+                };


Factor-out into a method - GetRandomInstance(Random rnd, int featuresLength) #Resolved

Factored out, made public with XML tags. Cannot add a featuresLength at this point (see above comment).

In reply to: 256542015 [](ancestors = 256542015)

glebuk · 2019-02-13T19:04:23Z

test/Microsoft.ML.Functional.Tests/Datasets/AllTypes.cs

+            return mlContext.Data.CreateTextLoader(
+                    new[] {
+                        new TextLoader.Column("Label", DataKind.Bool, 0),
+                        new TextLoader.Column("Features", DataKind.R4, 1, 5),


5 [](start = 74, length = 1)

factor out as param. Do same with the GetRandomINstance() #ByDesign

I can't add this as a parameter unless there is a way to alter the class attributes dynamically. For our tests, we need LoadColumn attributes to point to the corresponding serialization column.

In reply to: 256542590 [](ancestors = 256542590)

glebuk · 2019-02-13T23:44:38Z

test/Microsoft.ML.Functional.Tests/Datasets/TypeTestData.cs

+        /// <returns></returns>
+        public static TypeTestData GetRandomInstance(Random rng)
+        {
+            if (rng == null)


if (rng == null) [](start = 12, length = 16)

We usually use Check()?

glebuk

Rogan Carr added 10 commits February 6, 2019 14:57

Updating docstrings

7557a83

Merge remote-tracking branch 'upstream/master'

e27e20f

Merge remote-tracking branch 'upstream/master'

22c2f1f

Merge remote-tracking branch 'upstream/master'

de92547

Merge remote-tracking branch 'upstream/master'

d43cf49

Merge remote-tracking branch 'upstream/master'

6dd83c1

Merge remote-tracking branch 'upstream/master'

82ace78

Merge remote-tracking branch 'upstream/master'

a178861

Adding functional tests for DataIO

4829f7a

Remove Tests FileSaver API test.

4da79e2

glebuk reviewed Feb 13, 2019

View reviewed changes

Addresing PR comments

aaf2a91

sfilipi reviewed Feb 13, 2019

View reviewed changes

sfilipi approved these changes Feb 13, 2019

View reviewed changes

glebuk reviewed Feb 13, 2019

View reviewed changes

Addressing PR comments.

8e07097

Updating TypeTest to have a variable floatarray

ab2db38

glebuk reviewed Feb 13, 2019

View reviewed changes

glebuk approved these changes Feb 13, 2019

View reviewed changes

Merge branch 'master' into 2499_datasources_scenario_tests

da95bc4

rogancarr merged commit 82fd7ea into dotnet:master Feb 14, 2019

rogancarr deleted the 2499_datasources_scenario_tests branch February 14, 2019 16:49

rogancarr mentioned this pull request Feb 15, 2019

V1 Scenarios need to be covered by tests #2498

Open

ghost locked as resolved and limited conversation to collaborators Mar 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Functional Tests for Data I/O #2518

Add Functional Tests for Data I/O #2518

rogancarr commented Feb 12, 2019

glebuk Feb 13, 2019 •

edited

Loading

rogancarr Feb 13, 2019

rogancarr Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

rogancarr Feb 13, 2019 •

edited

Loading

sfilipi Feb 13, 2019

glebuk Feb 13, 2019 •

edited

Loading

rogancarr Feb 13, 2019

glebuk Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

rogancarr Feb 13, 2019

glebuk Feb 13, 2019 •

edited

Loading

rogancarr Feb 13, 2019

codecov bot commented Feb 13, 2019

codecov bot commented Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

rogancarr Feb 13, 2019

TomFinley commented Feb 13, 2019

rogancarr commented Feb 13, 2019

sfilipi Feb 13, 2019 •

edited by rogancarr

Loading

sfilipi left a comment

glebuk Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited by rogancarr

Loading

rogancarr Feb 13, 2019

glebuk Feb 13, 2019 •

edited by rogancarr

Loading

rogancarr Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019

glebuk left a comment

Add Functional Tests for Data I/O #2518

Add Functional Tests for Data I/O #2518

Conversation

rogancarr commented Feb 12, 2019

glebuk Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rogancarr Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

glebuk Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

rogancarr Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glebuk Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glebuk Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

glebuk Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

glebuk Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glebuk Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 13, 2019

Codecov Report

codecov bot commented Feb 13, 2019 • edited Loading

Codecov Report

glebuk Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomFinley commented Feb 13, 2019

rogancarr commented Feb 13, 2019

sfilipi Feb 13, 2019 • edited by rogancarr Loading

Choose a reason for hiding this comment

sfilipi left a comment

Choose a reason for hiding this comment

glebuk Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

glebuk Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

glebuk Feb 13, 2019 • edited by rogancarr Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glebuk Feb 13, 2019 • edited by rogancarr Loading

Choose a reason for hiding this comment

rogancarr Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glebuk left a comment

Choose a reason for hiding this comment

glebuk Feb 13, 2019 •

edited

Loading

rogancarr Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

rogancarr Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

codecov bot commented Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

sfilipi Feb 13, 2019 •

edited by rogancarr

Loading

glebuk Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited

Loading

glebuk Feb 13, 2019 •

edited by rogancarr

Loading

glebuk Feb 13, 2019 •

edited by rogancarr

Loading

rogancarr Feb 13, 2019 •

edited

Loading