Remove Legacy dependency in ML.FSharp.Tests #2021
Conversation
| */ | ||
| #pragma warning disable 612 | ||
| [Fact(Skip = "Missing data set. See https://github.com/dotnet/machinelearning/issues/3")] | ||
| public async void PredictHousePriceModelTest() |
There was a problem hiding this comment.
In my upcoming PR, I am deleting this test .
This test is being skipped so not not adding to code coverage. Also, I believe this test was testing the model load and prediction path.
In your PR, you are using a model generated using doing .Fit() but there is already an existing test for it HousePriceTrainAndPredictionTests.cs
@codemzs . Thoughts ?
#Resolved
There was a problem hiding this comment.
I agree that these tests can be deleted, I did not see the HousePriceTrainAndPreictionTests.cs file. I am going to remove this test then
| let ml = MLContext(seed = new System.Nullable<int>(1), conc = 1) | ||
| let data = ml.Data.ReadFromTextFile<SentimentData>(testDataPath, hasHeader = true) | ||
|
|
||
| let pipeline = ml.Transforms.Text.FeaturizeText("SentimentText", "Features") |
There was a problem hiding this comment.
MinDocumentsInLeafts is no longer available in the new APIs?
There was a problem hiding this comment.
By default it is set to 10 (instead of 2 in the old test). I am not sure how to create an Action object in F#, and I believe this will not change the code coverage. We have tests that are specific to FastTree for binary classification that set the MinDocumentsInLeafs to something different.
artidoro
changed the title
ghost
locked as resolved and limited conversation to collaborators
4 participants
Part of the effort described in #1971.
This PR removes Legacy code in ML.TestFramework and ML.FSharp.Tests, and replaces it with the new Estimator, Transformer API. Specifically, it changes tests that were using ModelHelper.cs (although those tests were skipped anyway as the dataset is missing). It also changes the F# code in the F# smoke tests to use the new API.
I checked the code coverage effect of this PR, and it increases the coverage by 0.02% (possibly for deleting test utility code that was not used).
EDIT:
We decided to remove the tests relying on the housing dataset in PR #2024. I am therefore reverting my changes on those tests and letting that PR remove all references to Legacy in the TestFramework DLL.