More namespace alignment #2724

sfilipi · 2019-02-25T23:49:56Z

Closes #2326 by :
1- First commit
substituting :

Microsoft.ML.Trainers.Ensemble.DiversityMeasure
Microsoft.ML.Trainers.Ensemble.FeatureSelector
Microsoft.ML.Trainers.Ensemble.SubModelSelector

with:
Microsoft.ML.Trainers.Ensemble

2 - Second commit:
Microsoft.ML.FactorizationMachine becomes Microsoft.ML.Trainers.FactorizationMachine
Microsoft.ML.EntryPoints.JsonUtils becomes Microsoft.ML.EntryPoints

3- third commit
Microsoft.ML.ImageAnalytics.EntryPoints becomes Microsoft.ML.ImageAnalytics
Microsoft.ML.Internal.Calibration becomes Microsoft.Ml.Calibrator

4- Fourth commit
Microsoft.ML.StaticPipe.Runtime changes to Microsoft.ML.StaticPipe

easier to review commit by commit.

I have used the VS replace all + reordering namespaces through Ctrl+R+G

eerhardt · 2019-02-26T15:23:39Z

src/Microsoft.ML.FastTree/RandomForestClassification.cs

@@ -10,7 +10,6 @@
 using Microsoft.ML.CommandLine;
 using Microsoft.ML.Data;
 using Microsoft.ML.EntryPoints;
-using Microsoft.ML.Internal.Calibration;
 using Microsoft.ML.Internal.Internallearn;


I assume Internal.Internallearn will be taken care of in another change.

Yep, I’ll be taking care of that today.

wschin · 2019-02-26T19:39:14Z

src/Microsoft.ML.ImageAnalytics/EntryPoints/ImageAnalytics.cs


 [assembly: LoadableClass(typeof(void), typeof(ImageAnalytics), null, typeof(SignatureEntryPointModule), "ImageAnalytics")]
-namespace Microsoft.ML.ImageAnalytics.EntryPoints
+namespace Microsoft.ML.ImageAnalytics
 {
    internal static class ImageAnalytics


ImageAnalytics [](start = 26, length = 14)

Humm, is it really good to have a class named by its namespace? Once I ran into missing-reference problem because of that. #Resolved

TomFinley · 2019-02-26T19:43:22Z

src/Microsoft.ML.Data/Prediction/Calibrator.cs

@@ -80,7 +79,7 @@
 [assembly: EntryPointModule(typeof(PavCalibratorTrainerFactory))]
 [assembly: EntryPointModule(typeof(PlattCalibratorTrainerFactory))]

-namespace Microsoft.ML.Internal.Calibration
+namespace Microsoft.ML.Calibrator


Calibrator [](start = 23, length = 10)

If we really want to have another namespace that's fine I guess, but if the namespace name is Microsoft.ML.Trainers then this should probably be Microsoft.ML.Calibrators, not Calibrator. #Resolved

Should we do Microsoft.ML.StaticPipe"s"? #Pending

let's do that as part of:
#2746

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

TomFinley · 2019-02-26T19:43:36Z

namespace Microsoft.ML.Calibrator

Similar here. #Resolved

Refers to: src/Microsoft.ML.Data/Prediction/CalibratorCatalog.cs:23 in a0eb13d. [](commit_id = a0eb13d, deletion_comment = False)

wschin

Overall LGTM. Just a minor comment about potential naming.

TomFinley

🕐

revoking review

singlis · 2019-02-26T20:53:11Z

namespace Microsoft.ML.LightGBM.StaticPipe

Should the namespace for this be Microsoft.ML.StaticPipe? Looking at other examples, classes are using Microsoft.ML.StaticPipeline for the namespace. #Pending

Refers to: src/Microsoft.ML.LightGBM.StaticPipe/LightGbmStaticExtensions.cs:11 in 2af661d. [](commit_id = 2af661d, deletion_comment = False)

singlis · 2019-02-26T20:53:37Z

namespace Microsoft.ML.LightGBM.StaticPipe

Are there other classes in the StaticPipe that don't use Microsoft.ML.StaticPipe?

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

Refers to: src/Microsoft.ML.LightGBM.StaticPipe/LightGbmStaticExtensions.cs:11 in 2af661d. [](commit_id = 2af661d, deletion_comment = False)

singlis · 2019-02-26T20:58:11Z

namespace Microsoft.ML.LightGBM.StaticPipe

Some other variations that I found:
Microsoft.ML.StaticPipe.Runtime
Microsoft.ML.HalLearners.StaticPipe
Microsoft.ML.Transforms.StaticPipe

In reply to: 467609182 [](ancestors = 467609182,467609030)

Refers to: src/Microsoft.ML.LightGBM.StaticPipe/LightGbmStaticExtensions.cs:11 in 2af661d. [](commit_id = 2af661d, deletion_comment = False)

singlis

singlis · 2019-02-26T21:02:35Z

Looks good to me, I left some feedback.

Microsoft.ML.Trainers.Ensemble.DiversityMeasure Microsoft.ML.Trainers.Ensemble.FeatureSelector Microsoft.ML.Trainers.Ensemble.SubModelSelector

and Microsoft.ML.FactorizationMachine -> Microsoft.ML.Trainers.FactorizationMachine

Microsoft.ML.Internal.Calibration -> Microsoft.Ml.Calibrator

sfilipi · 2019-02-26T22:00:06Z

namespace Microsoft.ML.LightGBM.StaticPipe

let me address that on a separate issue/PR since it is not related to the core Microsoft.ML nuget.

Logged: #2746

In reply to: 467610688 [](ancestors = 467610688,467609182,467609030)

Refers to: src/Microsoft.ML.LightGBM.StaticPipe/LightGbmStaticExtensions.cs:11 in 2af661d. [](commit_id = 2af661d, deletion_comment = False)

Microsoft.ML.Calibrators namespace for calibrators Microsoft.ML.Data.Evaluators.Metrics -> Microsoft.ML.Data Microsoft.ML.FastTree --> Microsoft.ML.Trainers.FastTree ICanSaveModel, ModelSaveContext move from Microsoft.ML.Model to Microsoft.Ml

…ugh cmd doesn't build the test folder. Extending the changes of the prior commit to the test folder

… and regenerating the catalog.

codecov · 2019-02-27T00:21:36Z

Codecov Report

Merging #2724 into master will increase coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2724      +/-   ##
==========================================
+ Coverage   71.65%   71.66%   +<.01%     
==========================================
  Files         808      808              
  Lines      142350   142350              
  Branches    16121    16121              
==========================================
+ Hits       102008   102010       +2     
+ Misses      35905    35903       -2     
  Partials     4437     4437

Flag	Coverage Δ
#Debug	`71.66% <0%> (ø)`	⬆️
#production	`67.91% <0%> (ø)`	⬆️
#test	`85.82% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
src/Microsoft.ML.StaticPipe/OnlineLearnerStatic.cs	`49.36% <ø> (ø)`	⬆️
...ft.ML.StandardLearners/Standard/MultiClass/Pkpd.cs	`88.23% <ø> (ø)`	⬆️
...ModelSelector/BestPerformanceRegressionSelector.cs	`8.33% <ø> (ø)`	⬆️
...ctor/SubModelSelector/BestDiverseSelectorBinary.cs	`10% <ø> (ø)`	⬆️
...Selector/SubModelSelector/AllSelectorMultiClass.cs	`80% <ø> (ø)`	⬆️
...rc/Microsoft.ML.Data/EntryPoints/EntryPointNode.cs	`68.58% <ø> (ø)`	⬆️
test/Microsoft.ML.Functional.Tests/Common.cs	`98.54% <ø> (ø)`	⬆️
...ML.LightGBM.StaticPipe/LightGbmStaticExtensions.cs	`46.42% <ø> (ø)`	⬆️
...osoft.ML.Data/DataLoadSave/Binary/UnsafeTypeOps.cs	`71.55% <ø> (ø)`	⬆️
src/Microsoft.ML.StaticPipe/Transformer.cs	`73.68% <ø> (ø)`	⬆️
... and 125 more

eerhardt · 2019-02-27T15:12:28Z

~~@Ivanidzo4ka @sfilipi -~~

~~From our OneNote yesterday we have:~~

	5- Microsoft.ML.FastTree -> Going to Microsoft.ML.Trainers.FastTree :QuantileRegressionTree, QuantileRegressionTreeEnsemble, RegressionTree, RegressionTreeBase, RegressionTreeEnsemble, TreeEnsemble -->
done PR: https://github.com/dotnet/machinelearning/pull/2724

~~I don't see that happening in this PR. Should we open a new issue for that task, since it wasn't done here?~~

Scratch all this. I was looking in the wrong place... 😕

sfilipi requested review from codemzs, shmoradims, wschin, TomFinley and Ivanidzo4ka February 25, 2019 23:50

sfilipi self-assigned this Feb 25, 2019

sfilipi added this to the 0219 milestone Feb 25, 2019

sfilipi added the API Issues pertaining the friendly API label Feb 25, 2019

sfilipi requested a review from eerhardt February 26, 2019 05:37

eerhardt reviewed Feb 26, 2019

View reviewed changes

wschin reviewed Feb 26, 2019

View reviewed changes

TomFinley reviewed Feb 26, 2019

View reviewed changes

wschin approved these changes Feb 26, 2019

View reviewed changes

TomFinley previously requested changes Feb 26, 2019

View reviewed changes

singlis approved these changes Feb 26, 2019

View reviewed changes

sfilipi added 4 commits February 26, 2019 13:36

removing :

2301bb8

Microsoft.ML.Trainers.Ensemble.DiversityMeasure Microsoft.ML.Trainers.Ensemble.FeatureSelector Microsoft.ML.Trainers.Ensemble.SubModelSelector

Microsoft.ML.EntryPoints.JsonUtils -> Microsoft.ML.EntryPoints

317ac60

and Microsoft.ML.FactorizationMachine -> Microsoft.ML.Trainers.FactorizationMachine

Microsoft.ML.ImageAnalytics.EntryPoints -> Microsoft.ML.ImageAnalytics

8c580ea

Microsoft.ML.Internal.Calibration -> Microsoft.Ml.Calibrator

Microsoft.ML.StaticPipe.Runtime -> Microsoft.ML.StaticPipe

12ab036

PR review comments + the following changes:

0e365de

Microsoft.ML.Calibrators namespace for calibrators Microsoft.ML.Data.Evaluators.Metrics -> Microsoft.ML.Data Microsoft.ML.FastTree --> Microsoft.ML.Trainers.FastTree ICanSaveModel, ModelSaveContext move from Microsoft.ML.Model to Microsoft.Ml

sfilipi force-pushed the moreNamespaceAlignment branch from a0eb13d to 0e365de Compare February 26, 2019 22:37

sfilipi added 2 commits February 26, 2019 14:53

the target of my replace all was the source folder, and building thro…

e1236d8

…ugh cmd doesn't build the test folder. Extending the changes of the prior commit to the test folder

Ivan's suggestion to move VersionInfo and ModelHeader to Microsoft.ML…

f8a7d87

… and regenerating the catalog.

sfilipi merged commit 2a1f2bb into dotnet:master Feb 26, 2019

sfilipi deleted the moreNamespaceAlignment branch February 26, 2019 23:57

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

More namespace alignment #2724

More namespace alignment #2724

sfilipi commented Feb 25, 2019 •

edited

Loading

eerhardt Feb 26, 2019

codemzs Feb 26, 2019

wschin Feb 26, 2019 •

edited by sfilipi

Loading

TomFinley Feb 26, 2019 •

edited by sfilipi

Loading

wschin Feb 26, 2019 •

edited by sfilipi

Loading

sfilipi Feb 26, 2019

TomFinley commented Feb 26, 2019 •

edited by sfilipi

Loading

wschin left a comment

TomFinley left a comment

singlis commented Feb 26, 2019 •

edited by sfilipi

Loading

singlis commented Feb 26, 2019

singlis commented Feb 26, 2019

singlis left a comment

singlis commented Feb 26, 2019

sfilipi commented Feb 26, 2019

codecov bot commented Feb 27, 2019 •

edited

Loading

eerhardt commented Feb 27, 2019 •

edited

Loading

More namespace alignment #2724

More namespace alignment #2724

Conversation

sfilipi commented Feb 25, 2019 • edited Loading

eerhardt Feb 26, 2019

Choose a reason for hiding this comment

codemzs Feb 26, 2019

Choose a reason for hiding this comment

wschin Feb 26, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

TomFinley Feb 26, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

wschin Feb 26, 2019 • edited by sfilipi Loading

Choose a reason for hiding this comment

sfilipi Feb 26, 2019

Choose a reason for hiding this comment

TomFinley commented Feb 26, 2019 • edited by sfilipi Loading

wschin left a comment

Choose a reason for hiding this comment

TomFinley left a comment

Choose a reason for hiding this comment

singlis commented Feb 26, 2019 • edited by sfilipi Loading

singlis commented Feb 26, 2019

singlis commented Feb 26, 2019

singlis left a comment

Choose a reason for hiding this comment

singlis commented Feb 26, 2019

sfilipi commented Feb 26, 2019

codecov bot commented Feb 27, 2019 • edited Loading

Codecov Report

eerhardt commented Feb 27, 2019 • edited Loading

sfilipi commented Feb 25, 2019 •

edited

Loading

wschin Feb 26, 2019 •

edited by sfilipi

Loading

TomFinley Feb 26, 2019 •

edited by sfilipi

Loading

wschin Feb 26, 2019 •

edited by sfilipi

Loading

TomFinley commented Feb 26, 2019 •

edited by sfilipi

Loading

singlis commented Feb 26, 2019 •

edited by sfilipi

Loading

codecov bot commented Feb 27, 2019 •

edited

Loading

eerhardt commented Feb 27, 2019 •

edited

Loading