Skip to content

Hide some duplicate methods #2904

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

Conversation

Ivanidzo4ka
Copy link
Contributor

fixes #2897

@codecov
Copy link

codecov bot commented Mar 11, 2019

Codecov Report

Merging #2904 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2904      +/-   ##
==========================================
+ Coverage    71.8%   71.81%   +<.01%     
==========================================
  Files         812      812              
  Lines      142644   142641       -3     
  Branches    16090    16090              
==========================================
+ Hits       102432   102433       +1     
+ Misses      35828    35825       -3     
+ Partials     4384     4383       -1
Flag Coverage Δ
#Debug 71.81% <ø> (ø) ⬆️
#production 67.95% <ø> (ø) ⬆️
#test 86.24% <ø> (ø) ⬆️
Impacted Files Coverage Δ
...Microsoft.ML.Data/DataLoadSave/TransformerChain.cs 87.11% <ø> (ø) ⬆️
.../Microsoft.ML.Data/Model/ModelOperationsCatalog.cs 100% <ø> (+16.66%) ⬆️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs 85.69% <0%> (+0.16%) ⬆️

@TomFinley
Copy link
Contributor

This seems like it intersects pretty closely with #2858 which as seen will be changing this API rather entirely to the point where what is noted here may be obviated. Should we instead focus on that one?

Copy link
Member

@ganik ganik left a comment

Choose a reason for hiding this comment

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

:shipit:

@Ivanidzo4ka Ivanidzo4ka merged commit e2464f6 into dotnet:master Mar 12, 2019
@Ivanidzo4ka
Copy link
Contributor Author

@TomFinley I've check #2858 and SaveTo remain public and CreatePredictEngine remains in two places.
So I would prefer to check this one, and let @yaeldekel decide should we continue to have two public CreatePredictionEngine or we need only one.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 23, 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.

Two Ways to Save a Model
4 participants