Added SQLite database to test loading of datasets in non-Windows builds #5080
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mstfbl
commented
May 1, 2020
mstfbl
requested review from
frank-dong-ms-zz,
harishsk,
antoniovs1029 and
Lynx1820
Codecov Report
@@ Coverage Diff @@
## master #5080 +/- ##
==========================================
- Coverage 75.66% 75.64% -0.02%
==========================================
Files 993 993
Lines 178157 178192 +35
Branches 19125 19125
==========================================
- Hits 134800 134799 -1
- Misses 38136 38164 +28
- Partials 5221 5229 +8
|
harishsk
reviewed
May 1, 2020
This was referenced May 1, 2020
Codecov Report
@@ Coverage Diff @@
## master #5080 +/- ##
==========================================
- Coverage 75.66% 75.64% -0.02%
==========================================
Files 993 993
Lines 178157 178192 +35
Branches 19125 19125
==========================================
- Hits 134800 134799 -1
- Misses 38136 38164 +28
- Partials 5221 5229 +8
|
|
Currently tests on MacOS and Linux builds are failing as the version of |
harishsk
reviewed
May 2, 2020
harishsk
reviewed
May 2, 2020
|
Marking this PR as draft for now, until the NuGet package for the up-to-date NuGet package for the |
mstfbl
commented
May 4, 2020
| @@ -20,6 +20,7 @@ | |||
| https://dotnet.myget.org/F/dotnet-core/api/v3/index.json; | |||
| https://dotnet.myget.org/F/roslyn-analyzers/api/v3/index.json; | |||
| https://pkgs.dev.azure.com/dnceng/public/_packaging/MachineLearning/nuget/v3/index.json; | |||
| https://pkgs.dev.azure.com/dnceng/public/_packaging/machinelearning-testdata/nuget/v3/index.json; | |||
There was a problem hiding this comment.
This change is necessary to obtain the most up-to-date NuGet package for TestDatasets.
mstfbl
commented
May 4, 2020
| return new DatabaseSource( | ||
| SqlClientFactory.Instance, | ||
| GetMSSQLConnectionString(TestDatasets.irisDb.name), | ||
| String.Format(command, $@"""{TestDatasets.irisDb.trainFilename}""")); |
There was a problem hiding this comment.
Passing TestDatasets.irisDb.trainFilename as $@"""{TestDatasets.irisDb.trainFilename}""" is necessary to correctly pass the table name as \"iris-train\" in MSSQL.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #4156 and related to #4175
MSSQL databases are not supported on Mac and Linux builds. As such, I've generated an equivalent SQLite database, and re-activated these disabled Iris tests so that these unit tests can run on non-Windows builds and produce equivalent test results.
Reactivated tests on Linux and MacOS list:
IrisLightGbm()IrisLightGbmWithLoadColumnName()IrisVectorLightGbm()IrisVectorLightGbmWithLoadColumnName()IrisSdcaMaximumEntropy()