Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor of OnnxConversionTests.cs #5185

Merged
merged 2 commits into from
Jun 2, 2020

Conversation

michaelgsharp
Copy link
Member

The majority of the tests in OnnxConversionTests.cs do the same thing. Build an ML.Net pipeline, export it to ONNX, and compare the results. This PR refactors the OnnxConversionTests.cs file to use a common method for all the tests that do that same functionality, removing about 400 lines of duplicate code. There were a few tests with functionality not exactly the same, and those were left as is.

…ts that can, removing about 400 lines of duplicate code.
@michaelgsharp michaelgsharp requested a review from a team June 1, 2020 21:11
@michaelgsharp michaelgsharp self-assigned this Jun 1, 2020
@codecov
Copy link

codecov bot commented Jun 1, 2020

Codecov Report

Merging #5185 into master will decrease coverage by 0.03%.
The diff coverage is 97.50%.

@@            Coverage Diff             @@
##           master    #5185      +/-   ##
==========================================
- Coverage   73.32%   73.29%   -0.04%     
==========================================
  Files        1007     1007              
  Lines      188386   188016     -370     
  Branches    20286    20257      -29     
==========================================
- Hits       138126   137797     -329     
+ Misses      44736    44693      -43     
- Partials     5524     5526       +2     
Flag Coverage Δ
#Debug 73.29% <97.50%> (-0.04%) ⬇️
#production 69.03% <ø> (+0.02%) ⬆️
#test 87.44% <97.50%> (-0.11%) ⬇️
Impacted Files Coverage Δ
test/Microsoft.ML.Tests/OnnxConversionTest.cs 98.96% <97.50%> (-0.24%) ⬇️
...osoft.ML.Transforms/Text/TokenizingByCharacters.cs 86.34% <0.00%> (-0.25%) ⬇️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs 85.78% <0.00%> (-0.16%) ⬇️
...soft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs 89.58% <0.00%> (+0.16%) ⬆️
test/Microsoft.ML.TestFramework/BaseTestClass.cs 86.76% <0.00%> (+0.40%) ⬆️
.../Microsoft.ML.Data/Prediction/CalibratorCatalog.cs 93.44% <0.00%> (+0.54%) ⬆️
...est/Microsoft.ML.TestFrameworkCommon/TestCommon.cs 82.10% <0.00%> (+1.05%) ⬆️
src/Microsoft.ML.Transforms/OneHotEncoding.cs 86.07% <0.00%> (+1.26%) ⬆️
src/Microsoft.ML.Maml/MAML.cs 26.21% <0.00%> (+1.45%) ⬆️
src/Microsoft.ML.Data/Data/Conversion.cs 78.68% <0.00%> (+1.69%) ⬆️
... and 3 more

Copy link
Contributor

@frank-dong-ms-zz frank-dong-ms-zz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@michaelgsharp michaelgsharp merged commit 06ab717 into dotnet:master Jun 2, 2020
@michaelgsharp michaelgsharp deleted the onnx-test-refactor branch June 2, 2020 05:31
@ghost ghost locked as resolved and limited conversation to collaborators Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants