Hide more of Microsoft.ML.Data (#2842)

* Get rid of unnecessary SubCatalogBase base class.

* Clean up unintentional usage of protected internal.

* ValueToKeyMappingEstimator.ColumnOptions now not publicly subclassable.

* Have the public abstract classes for estimator/transformers/etc. be practically unsubclassable.

* Internalize the infrastructure surrounding TransformWrapper.

* Internalize DefaultColumnNames utility class for storing const string literals.

* Use explicit IInternalCatalog, thereby avoiding SubCatalogBase in public surface.

* Simplify CatalogUtils.

* Copy documentation from IEstimator to two implementations of GetOutputSchema.

Author: TomFinley

src/Microsoft.ML.Data/Commands/DefaultColumnNames.cs

@@ -4,7 +4,13 @@
 
 namespace Microsoft.ML.Data
 {
-    public static class DefaultColumnNames
+    /// <summary>
+    /// A set of string literals intended to be "canonical" names for column names intended for particular purpose.
+    /// While not part of the public API surface, its primary purpose is intended to be used in such a way as to encourage
+    /// uniformity on the public API surface, wherever it is judged where columns with default names should be consumed.
+    /// </summary>
+    [BestFriend]
+    internal static class DefaultColumnNames
     {
         public const string Features = "Features";
         public const string Label = "Label";

src/Microsoft.ML.Data/DataLoadSave/DataOperationsCatalog.cs

@@ -14,15 +14,15 @@ namespace Microsoft.ML
     /// A catalog of operations over data that are not transformers or estimators.
     /// This includes data loaders, saving, caching, filtering etc.
     /// </summary>
-    public sealed class DataOperationsCatalog
+    public sealed class DataOperationsCatalog : IInternalCatalog
     {
-        [BestFriend]
-        internal IHostEnvironment Environment { get; }
+        IHostEnvironment IInternalCatalog.Environment => _env;
+        private readonly IHostEnvironment _env;
 
         internal DataOperationsCatalog(IHostEnvironment env)
         {
             Contracts.AssertValue(env);
-            Environment = env;
+            _env = env;
         }
 
         /// <summary>
@@ -52,9 +52,9 @@ internal DataOperationsCatalog(IHostEnvironment env)
         public IDataView LoadFromEnumerable<TRow>(IEnumerable<TRow> data, SchemaDefinition schemaDefinition = null)
             where TRow : class
         {
-            Environment.CheckValue(data, nameof(data));
-            Environment.CheckValueOrNull(schemaDefinition);
-            return DataViewConstructionUtils.CreateFromEnumerable(Environment, data, schemaDefinition);
+            _env.CheckValue(data, nameof(data));
+            _env.CheckValueOrNull(schemaDefinition);
+            return DataViewConstructionUtils.CreateFromEnumerable(_env, data, schemaDefinition);
         }
 
         /// <summary>
@@ -77,10 +77,10 @@ public IEnumerable<TRow> CreateEnumerable<TRow>(IDataView data, bool reuseRowObj
             bool ignoreMissingColumns = false, SchemaDefinition schemaDefinition = null)
             where TRow : class, new()
         {
-            Environment.CheckValue(data, nameof(data));
-            Environment.CheckValueOrNull(schemaDefinition);
+            _env.CheckValue(data, nameof(data));
+            _env.CheckValueOrNull(schemaDefinition);
 
-            var engine = new PipeEngine<TRow>(Environment, data, ignoreMissingColumns, schemaDefinition);
+            var engine = new PipeEngine<TRow>(_env, data, ignoreMissingColumns, schemaDefinition);
             return engine.RunPipe(reuseRowObject);
         }
 
@@ -109,9 +109,9 @@ public IDataView BootstrapSample(IDataView input,
             int? seed = null,
             bool complement = BootstrapSamplingTransformer.Defaults.Complement)
         {
-            Environment.CheckValue(input, nameof(input));
+            _env.CheckValue(input, nameof(input));
             return new BootstrapSamplingTransformer(
-                Environment,
+                _env,
                 input,
                 complement: complement,
                 seed: (uint?) seed,
@@ -139,16 +139,16 @@ public IDataView BootstrapSample(IDataView input,
         /// </example>
         public IDataView Cache(IDataView input, params string[] columnsToPrefetch)
         {
-            Environment.CheckValue(input, nameof(input));
-            Environment.CheckValueOrNull(columnsToPrefetch);
+            _env.CheckValue(input, nameof(input));
+            _env.CheckValueOrNull(columnsToPrefetch);
 
             int[] prefetch = new int[Utils.Size(columnsToPrefetch)];
             for (int i = 0; i < prefetch.Length; i++)
             {
                 if (!input.Schema.TryGetColumnIndex(columnsToPrefetch[i], out prefetch[i]))
-                    throw Environment.ExceptSchemaMismatch(nameof(columnsToPrefetch), "prefetch", columnsToPrefetch[i]);
+                    throw _env.ExceptSchemaMismatch(nameof(columnsToPrefetch), "prefetch", columnsToPrefetch[i]);
             }
-            return new CacheDataView(Environment, input, prefetch);
+            return new CacheDataView(_env, input, prefetch);
         }
 
         /// <summary>
@@ -171,14 +171,14 @@ public IDataView Cache(IDataView input, params string[] columnsToPrefetch)
         /// </example>
         public IDataView FilterRowsByColumn(IDataView input, string columnName, double lowerBound = double.NegativeInfinity, double upperBound = double.PositiveInfinity)
         {
-            Environment.CheckValue(input, nameof(input));
-            Environment.CheckNonEmpty(columnName, nameof(columnName));
-            Environment.CheckParam(lowerBound < upperBound, nameof(upperBound), "Must be less than lowerBound");
+            _env.CheckValue(input, nameof(input));
+            _env.CheckNonEmpty(columnName, nameof(columnName));
+            _env.CheckParam(lowerBound < upperBound, nameof(upperBound), "Must be less than lowerBound");
 
             var type = input.Schema[columnName].Type;
             if (!(type is NumberDataViewType))
-                throw Environment.ExceptSchemaMismatch(nameof(columnName), "filter", columnName, "number", type.ToString());
-            return new RangeFilter(Environment, input, columnName, lowerBound, upperBound, false);
+                throw _env.ExceptSchemaMismatch(nameof(columnName), "filter", columnName, "number", type.ToString());
+            return new RangeFilter(_env, input, columnName, lowerBound, upperBound, false);
         }
 
         /// <summary>
@@ -203,16 +203,16 @@ public IDataView FilterRowsByColumn(IDataView input, string columnName, double l
         /// </example>
         public IDataView FilterRowsByKeyColumnFraction(IDataView input, string columnName, double lowerBound = 0, double upperBound = 1)
         {
-            Environment.CheckValue(input, nameof(input));
-            Environment.CheckNonEmpty(columnName, nameof(columnName));
-            Environment.CheckParam(0 <= lowerBound && lowerBound <= 1, nameof(lowerBound), "Must be in [0, 1]");
-            Environment.CheckParam(0 <= upperBound && upperBound <= 2, nameof(upperBound), "Must be in [0, 2]");
-            Environment.CheckParam(lowerBound <= upperBound, nameof(upperBound), "Must be no less than lowerBound");
+            _env.CheckValue(input, nameof(input));
+            _env.CheckNonEmpty(columnName, nameof(columnName));
+            _env.CheckParam(0 <= lowerBound && lowerBound <= 1, nameof(lowerBound), "Must be in [0, 1]");
+            _env.CheckParam(0 <= upperBound && upperBound <= 2, nameof(upperBound), "Must be in [0, 2]");
+            _env.CheckParam(lowerBound <= upperBound, nameof(upperBound), "Must be no less than lowerBound");
 
             var type = input.Schema[columnName].Type;
             if (type.GetKeyCount() == 0)
-                throw Environment.ExceptSchemaMismatch(nameof(columnName), "filter", columnName, "KeyType", type.ToString());
-            return new RangeFilter(Environment, input, columnName, lowerBound, upperBound, false);
+                throw _env.ExceptSchemaMismatch(nameof(columnName), "filter", columnName, "KeyType", type.ToString());
+            return new RangeFilter(_env, input, columnName, lowerBound, upperBound, false);
         }
 
         /// <summary>
@@ -230,10 +230,10 @@ public IDataView FilterRowsByKeyColumnFraction(IDataView input, string columnNam
         /// </example>
         public IDataView FilterRowsByMissingValues(IDataView input, params string[] columns)
         {
-            Environment.CheckValue(input, nameof(input));
-            Environment.CheckUserArg(Utils.Size(columns) > 0, nameof(columns));
+            _env.CheckValue(input, nameof(input));
+            _env.CheckUserArg(Utils.Size(columns) > 0, nameof(columns));
 
-            return new NAFilter(Environment, input, complement: false, columns);
+            return new NAFilter(_env, input, complement: false, columns);
         }
 
         /// <summary>
@@ -268,8 +268,8 @@ public IDataView ShuffleRows(IDataView input,
             int shufflePoolSize = RowShufflingTransformer.Defaults.PoolRows,
             bool shuffleSource = !RowShufflingTransformer.Defaults.PoolOnly)
         {
-            Environment.CheckValue(input, nameof(input));
-            Environment.CheckUserArg(shufflePoolSize > 0, nameof(shufflePoolSize), "Must be positive");
+            _env.CheckValue(input, nameof(input));
+            _env.CheckUserArg(shufflePoolSize > 0, nameof(shufflePoolSize), "Must be positive");
 
             var options = new RowShufflingTransformer.Options
             {
@@ -279,7 +279,7 @@ public IDataView ShuffleRows(IDataView input,
                 ForceShuffleSeed = seed
             };
 
-            return new RowShufflingTransformer(Environment, options, input);
+            return new RowShufflingTransformer(_env, options, input);
         }
 
         /// <summary>
@@ -299,15 +299,15 @@ public IDataView ShuffleRows(IDataView input,
         /// </example>
         public IDataView SkipRows(IDataView input, long count)
         {
-            Environment.CheckValue(input, nameof(input));
-            Environment.CheckUserArg(count > 0, nameof(count), "Must be greater than zero.");
+            _env.CheckValue(input, nameof(input));
+            _env.CheckUserArg(count > 0, nameof(count), "Must be greater than zero.");
 
             var options = new SkipTakeFilter.SkipOptions()
             {
                 Count = count
             };
 
-            return new SkipTakeFilter(Environment, options, input);
+            return new SkipTakeFilter(_env, options, input);
         }
 
         /// <summary>
@@ -327,15 +327,15 @@ public IDataView SkipRows(IDataView input, long count)
         /// </example>
         public IDataView TakeRows(IDataView input, long count)
         {
-            Environment.CheckValue(input, nameof(input));
-            Environment.CheckUserArg(count > 0, nameof(count), "Must be greater than zero.");
+            _env.CheckValue(input, nameof(input));
+            _env.CheckUserArg(count > 0, nameof(count), "Must be greater than zero.");
 
             var options = new SkipTakeFilter.TakeOptions()
             {
                 Count = count
             };
 
-            return new SkipTakeFilter(Environment, options, input);
+            return new SkipTakeFilter(_env, options, input);
         }
     }
 }

src/Microsoft.ML.Data/DataLoadSave/FakeSchema.cs

@@ -13,6 +13,7 @@ namespace Microsoft.ML.Data.DataLoadSave
     /// It will pretend that all vector sizes are equal to 10, all key value counts are equal to 10,
     /// and all values are defaults (for annotations).
     /// </summary>
+    [BestFriend]
     internal static class FakeSchemaFactory
     {
         private const int AllVectorSizes = 10;

src/Microsoft.ML.Data/DataLoadSave/TransformWrapper.cs

@@ -15,9 +15,13 @@
 
 namespace Microsoft.ML.Data
 {
-    // REVIEW: this class is public, as long as the Wrappers.cs in tests still rely on it.
-    // It needs to become internal.
-    public sealed class TransformWrapper : ITransformer
+    /// <summary>
+    /// This is a shim class to present the legacy <see cref="IDataTransform"/> interface as an <see cref="ITransformer"/>.
+    /// Note that there are some important differences in usages that make this shimming somewhat non-seemless, so the goal
+    /// would be gradual removal of this as we do away with <see cref="IDataTransform"/> based code.
+    /// </summary>
+    [BestFriend]
+    internal sealed class TransformWrapper : ITransformer
     {
         internal const string LoaderSignature = "TransformWrapper";
         private const string TransformDirTemplate = "Step_{0:000}";
@@ -148,11 +152,13 @@ IRowToRowMapper ITransformer.GetRowToRowMapper(DataViewSchema inputSchema)
     /// <summary>
     /// Estimator for trained wrapped transformers.
     /// </summary>
-    public abstract class TrainedWrapperEstimatorBase : IEstimator<TransformWrapper>
+    internal abstract class TrainedWrapperEstimatorBase : IEstimator<TransformWrapper>
     {
-        protected readonly IHost Host;
+        [BestFriend]
+        private protected readonly IHost Host;
 
-        protected TrainedWrapperEstimatorBase(IHost host)
+        [BestFriend]
+        private protected TrainedWrapperEstimatorBase(IHost host)
         {
             Contracts.CheckValue(host, nameof(host));
             Host = host;

src/Microsoft.ML.Data/DataLoadSave/TrivialEstimator.cs

@@ -16,10 +16,13 @@ namespace Microsoft.ML.Data
     public abstract class TrivialEstimator<TTransformer> : IEstimator<TTransformer>
         where TTransformer : class, ITransformer
     {
-        protected readonly IHost Host;
-        protected readonly TTransformer Transformer;
+        [BestFriend]
+        private protected readonly IHost Host;
+        [BestFriend]
+        private protected readonly TTransformer Transformer;
 
-        protected TrivialEstimator(IHost host, TTransformer transformer)
+        [BestFriend]
+        private protected TrivialEstimator(IHost host, TTransformer transformer)
         {
             Contracts.AssertValue(host);
 

src/Microsoft.ML.Data/DataView/DataViewConstructionUtils.cs

@@ -740,7 +740,7 @@ public abstract partial class AnnotationInfo
 
         internal abstract Delegate GetGetterDelegate();
 
-        protected AnnotationInfo(string kind, DataViewType annotationType)
+        private protected AnnotationInfo(string kind, DataViewType annotationType)
         {
             Contracts.AssertValueOrNull(annotationType);
             Contracts.AssertNonEmpty(kind);

src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs

@@ -10,55 +10,46 @@ namespace Microsoft.ML
     /// <summary>
     /// An object serving as a 'catalog' of available model operations.
     /// </summary>
-    public sealed class ModelOperationsCatalog
+    public sealed class ModelOperationsCatalog : IInternalCatalog
     {
-        /// <summary>
-        /// This is a best friend because an extension method defined in another assembly needs this field.
-        /// </summary>
-        [BestFriend]
-        internal IHostEnvironment Environment { get; }
+        IHostEnvironment IInternalCatalog.Environment => _env;
+        private readonly IHostEnvironment _env;
 
         public ExplainabilityTransforms Explainability { get; }
 
         internal ModelOperationsCatalog(IHostEnvironment env)
         {
             Contracts.AssertValue(env);
-            Environment = env;
+            _env = env;
 
             Explainability = new ExplainabilityTransforms(this);
         }
 
-        public abstract class SubCatalogBase
-        {
-            internal IHostEnvironment Environment { get; }
-
-            protected SubCatalogBase(ModelOperationsCatalog owner)
-            {
-                Environment = owner.Environment;
-            }
-        }
-
         /// <summary>
         /// Save the model to the stream.
         /// </summary>
         /// <param name="model">The trained model to be saved.</param>
         /// <param name="stream">A writeable, seekable stream to save to.</param>
-        public void Save(ITransformer model, Stream stream) => model.SaveTo(Environment, stream);
+        public void Save(ITransformer model, Stream stream) => model.SaveTo(_env, stream);
 
         /// <summary>
         /// Load the model from the stream.
         /// </summary>
         /// <param name="stream">A readable, seekable stream to load from.</param>
         /// <returns>The loaded model.</returns>
-        public ITransformer Load(Stream stream) => TransformerChain.LoadFrom(Environment, stream);
+        public ITransformer Load(Stream stream) => TransformerChain.LoadFrom(_env, stream);
 
         /// <summary>
         /// The catalog of model explainability operations.
         /// </summary>
-        public sealed class ExplainabilityTransforms : SubCatalogBase
+        public sealed class ExplainabilityTransforms : IInternalCatalog
         {
-            internal ExplainabilityTransforms(ModelOperationsCatalog owner) : base(owner)
+            IHostEnvironment IInternalCatalog.Environment => _env;
+            private readonly IHostEnvironment _env;
+
+            internal ExplainabilityTransforms(ModelOperationsCatalog owner)
             {
+                _env = owner._env;
             }
         }
 
@@ -75,7 +66,7 @@ public PredictionEngine<TSrc, TDst> CreatePredictionEngine<TSrc, TDst>(ITransfor
             where TSrc : class
             where TDst : class, new()
         {
-            return new PredictionEngine<TSrc, TDst>(Environment, transformer, false, inputSchemaDefinition, outputSchemaDefinition);
+            return new PredictionEngine<TSrc, TDst>(_env, transformer, false, inputSchemaDefinition, outputSchemaDefinition);
         }
     }
 }

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

@@ -103,7 +103,7 @@ public abstract class PredictionEngineBase<TSrc, TDst> : IDisposable
         /// <summary>
         /// Provides output schema.
         /// </summary>
-        public DataViewSchema OutputSchema;
+        public DataViewSchema OutputSchema { get; }
 
         [BestFriend]
         private protected ITransformer Transformer { get; }
@@ -180,9 +180,11 @@ public TDst Predict(TSrc example)
             return result;
         }
 
-        protected void ExtractValues(TSrc example) => _inputRow.ExtractValues(example);
+        [BestFriend]
+        private protected void ExtractValues(TSrc example) => _inputRow.ExtractValues(example);
 
-        protected void FillValues(TDst prediction) => _outputRow.FillValues(prediction);
+        [BestFriend]
+        private protected void FillValues(TDst prediction) => _outputRow.FillValues(prediction);
 
         /// <summary>
         /// Run prediction pipeline on one example.