Mark EntryPoints classes and APIs as internal

[eerhardt]

We have a few classes in Microsoft.ML.Core that are public, but they shouldn't be.

machinelearning/src/Microsoft.ML.Core/EntryPoints/TransformModel.cs

Line 14 in 13b3339

public abstract class TransformModel

machinelearning/src/Microsoft.ML.Core/EntryPoints/PredictorModel.cs

Line 14 in 13b3339

public abstract class PredictorModel

Basically, anything under the Microsoft.ML.EntryPoints namespace should be internal.

This is OK, because anything that uses EntryPoints already has InternalsVisibleTo.

@TomFinley @glebuk @ganik @shauheen

[ganik]

Would we allow third parties to develop against Entrypoints ? If yes then it cant be internal.

[eerhardt]

My opinion is that Entry Points are not part of ML.NET's v1.0 public API. We can make them public later, when necessary. But for v1.0, the Estimator/Transformer APIs are the public APIs that third parties should develop against.

[eerhardt]

There are still public types in this namespace in the core package:

namespace Microsoft.ML.EntryPoints {
    public abstract class EvaluateInputBase {
        public IDataView Data;
        public string NameColumn;
        protected EvaluateInputBase();
    }
    public abstract class LearnerInputBase {
        public string FeatureColumn;
        protected LearnerInputBase();
    }
    public abstract class LearnerInputBaseWithGroupId : LearnerInputBaseWithWeight {
        public string GroupIdColumn;
        protected LearnerInputBaseWithGroupId();
    }
    public abstract class LearnerInputBaseWithLabel : LearnerInputBase {
        public string LabelColumn;
        protected LearnerInputBaseWithLabel();
    }
    public abstract class LearnerInputBaseWithWeight : LearnerInputBaseWithLabel {
        public string WeightColumn;
        protected LearnerInputBaseWithWeight();
    }
    public abstract class TransformInputBase {
        protected TransformInputBase();
    }
    public abstract class UnsupervisedLearnerInputBaseWithWeight : LearnerInputBase {
        public string WeightColumn;
        protected UnsupervisedLearnerInputBaseWithWeight();
    }
}

machinelearning/src/Microsoft.ML.Data/EntryPoints/InputBase.cs

Lines 15 to 121 in a16eb30

	/// <summary>
	/// The base class for all transform inputs.
	/// </summary>
	[TlcModule.EntryPointKind(typeof(CommonInputs.ITransformInput))]
	public abstract class TransformInputBase
	{
	/// <summary>
	/// The input dataset. Used only in entry-point methods, since the normal API mechanism for feeding in a dataset to
	/// create an <see cref="ITransformer"/> is to use the <see cref="IEstimator{TTransformer}.Fit(IDataView)"/> method.
	/// </summary>
	[BestFriend]
	[Argument(ArgumentType.Required, HelpText = "Input dataset", Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly, SortOrder = 1)]
	internal IDataView Data;
	}
	[BestFriend]
	internal enum CachingOptions
	{
	Auto,
	Memory,
	Disk,
	None
	}
	/// <summary>
	/// The base class for all learner inputs.
	/// </summary>
	[TlcModule.EntryPointKind(typeof(CommonInputs.ITrainerInput))]
	public abstract class LearnerInputBase
	{
	/// <summary>
	/// The data to be used for training. Used only in entry-points, since in the API the expected mechanism is
	/// that the user will use the <see cref="IEstimator{TTransformer}.Fit(IDataView)"/> or some other train
	/// method.
	/// </summary>
	[BestFriend]
	[Argument(ArgumentType.Required, ShortName = "data", HelpText = "The data to be used for training", SortOrder = 1, Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly)]
	internal IDataView TrainingData;
	/// <summary>
	/// Column to use for features.
	/// </summary>
	[Argument(ArgumentType.AtMostOnce, HelpText = "Column to use for features", ShortName = "feat", SortOrder = 2, Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly)]
	public string FeatureColumn = DefaultColumnNames.Features;
	/// <summary>
	/// Normalize option for the feature column. Used only in entry-points, since in the API the user is expected to do this themselves.
	/// </summary>
	[BestFriend]
	[Argument(ArgumentType.AtMostOnce, HelpText = "Normalize option for the feature column", ShortName = "norm", SortOrder = 5, Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly)]
	internal NormalizeOption NormalizeFeatures = NormalizeOption.Auto;
	/// <summary>
	/// Whether learner should cache input training data. Used only in entry-points, since the intended API mechanism
	/// is that the user will use the <see cref="DataOperationsCatalog.Cache(IDataView, string[])"/> or other method
	/// like <see cref="EstimatorChain{TLastTransformer}.AppendCacheCheckpoint(IHostEnvironment)"/>.
	/// </summary>
	[BestFriend]
	[Argument(ArgumentType.LastOccurenceWins, HelpText = "Whether learner should cache input training data", ShortName = "cache", SortOrder = 6, Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly)]
	internal CachingOptions Caching = CachingOptions.Auto;
	}
	/// <summary>
	/// The base class for all learner inputs that support a Label column.
	/// </summary>
	[TlcModule.EntryPointKind(typeof(CommonInputs.ITrainerInputWithLabel))]
	public abstract class LearnerInputBaseWithLabel : LearnerInputBase
	{
	/// <summary>
	/// Column to use for labels.
	/// </summary>
	[Argument(ArgumentType.AtMostOnce, HelpText = "Column to use for labels", ShortName = "lab", SortOrder = 3, Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly)]
	public string LabelColumn = DefaultColumnNames.Label;
	}
	// REVIEW: This is a known antipattern, but the solution involves the decorator pattern which can't be used in this case.
	/// <summary>
	/// The base class for all learner inputs that support a weight column.
	/// </summary>
	[TlcModule.EntryPointKind(typeof(CommonInputs.ITrainerInputWithWeight))]
	public abstract class LearnerInputBaseWithWeight : LearnerInputBaseWithLabel
	{
	/// <summary>
	/// The name of the example weight column.
	/// </summary>
	[Argument(ArgumentType.AtMostOnce, HelpText = "Column to use for example weight", ShortName = "weight", SortOrder = 4, Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly)]
	public string WeightColumn = null;
	}
	/// <summary>
	/// The base class for all unsupervised learner inputs that support a weight column.
	/// </summary>
	[TlcModule.EntryPointKind(typeof(CommonInputs.IUnsupervisedTrainerWithWeight))]
	public abstract class UnsupervisedLearnerInputBaseWithWeight : LearnerInputBase
	{
	/// <summary>
	/// Column to use for example weight.
	/// </summary>
	[Argument(ArgumentType.AtMostOnce, HelpText = "Column to use for example weight", ShortName = "weight", SortOrder = 4, Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly)]
	public string WeightColumn = null;
	}
	/// <summary>
	/// The base class for all evaluators inputs.
	/// </summary>
	[TlcModule.EntryPointKind(typeof(CommonInputs.IEvaluatorInput))]
	public abstract class EvaluateInputBase

[Ivanidzo4ka]

We can't make them not public.
Don't you think this is more a issue of correct namespace?
We have separate issue regarding fixing namespaces, can we handle that in that issue rather than in this one?

[Ivanidzo4ka]

Actually, i think EvaluateInputBase can be make internal. Everything related to LearnerInputBase probably not.

[eerhardt]

My main concern with this issue is:

Basically, anything under the `Microsoft.ML.EntryPoints` namespace should be internal.

If there are types that cannot be made internal in this namespace, then they should not be in this namespace.