Use explicit IInternalCatalog, thereby avoiding SubCatalogBase in public surface. Simplify CatalogUtils.

Author: TomFinley

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/Model/ModelOperationsCatalog.cs

@@ -10,20 +10,17 @@ 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);
         }
@@ -33,25 +30,26 @@ internal ModelOperationsCatalog(IHostEnvironment env)
         /// </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
+        public sealed class ExplainabilityTransforms : IInternalCatalog
         {
-            internal IHostEnvironment Environment { get; }
+            IHostEnvironment IInternalCatalog.Environment => _env;
+            private readonly IHostEnvironment _env;
 
             internal ExplainabilityTransforms(ModelOperationsCatalog owner)
             {
-                Environment = owner.Environment;
+                _env = owner._env;
             }
         }
 
@@ -68,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/TrainCatalog.cs

@@ -16,10 +16,12 @@ namespace Microsoft.ML
     /// "area" of machine learning. A subclass would represent a particular task in machine learning. The idea
     /// is that a user can instantiate that particular area, and get trainers and evaluators.
     /// </summary>
-    public abstract class TrainCatalogBase
+    public abstract class TrainCatalogBase : IInternalCatalog
     {
+        IHostEnvironment IInternalCatalog.Environment => Environment;
+
         [BestFriend]
-        internal IHostEnvironment Environment { get; }
+        private protected IHostEnvironment Environment { get; }
 
         /// <summary>
         /// A pair of datasets, for the train and test set.
@@ -245,8 +247,10 @@ private void EnsureGroupPreservationColumn(ref IDataView data, ref string sampli
         /// through <see cref="CatalogUtils"/> to get more "hidden" information from this object,
         /// for example, the environment.
         /// </summary>
-        public abstract class CatalogInstantiatorBase
+        public abstract class CatalogInstantiatorBase : IInternalCatalog
         {
+            IHostEnvironment IInternalCatalog.Environment => Owner.GetEnvironment();
+
             [BestFriend]
             internal TrainCatalogBase Owner { get; }
 
@@ -402,7 +406,7 @@ public NaiveCalibratorEstimator Naive(
                   string labelColumnName = DefaultColumnNames.Label,
                   string scoreColumnName = DefaultColumnNames.Score)
             {
-                return new NaiveCalibratorEstimator(Owner.Environment, labelColumnName, scoreColumnName);
+                return new NaiveCalibratorEstimator(Owner.GetEnvironment(), labelColumnName, scoreColumnName);
             }
             /// <summary>
             /// Adds probability column by training <a href="https://en.wikipedia.org/wiki/Platt_scaling">platt calibrator</a>.
@@ -422,7 +426,7 @@ public PlattCalibratorEstimator Platt(
                   string scoreColumnName = DefaultColumnNames.Score,
                   string exampleWeightColumnName = null)
             {
-                return new PlattCalibratorEstimator(Owner.Environment, labelColumnName, scoreColumnName, exampleWeightColumnName);
+                return new PlattCalibratorEstimator(Owner.GetEnvironment(), labelColumnName, scoreColumnName, exampleWeightColumnName);
             }
 
             /// <summary>
@@ -443,7 +447,7 @@ public FixedPlattCalibratorEstimator Platt(
                 double offset,
                 string scoreColumnName = DefaultColumnNames.Score)
             {
-                return new FixedPlattCalibratorEstimator(Owner.Environment, slope, offset, scoreColumnName);
+                return new FixedPlattCalibratorEstimator(Owner.GetEnvironment(), slope, offset, scoreColumnName);
             }
 
             /// <summary>
@@ -468,7 +472,7 @@ public IsotonicCalibratorEstimator Isotonic(
                 string scoreColumnName = DefaultColumnNames.Score,
                 string exampleWeightColumnName = null)
             {
-                return new IsotonicCalibratorEstimator(Owner.Environment, labelColumnName, scoreColumnName, exampleWeightColumnName);
+                return new IsotonicCalibratorEstimator(Owner.GetEnvironment(), labelColumnName, scoreColumnName, exampleWeightColumnName);
             }
         }
     }

src/Microsoft.ML.Data/Transforms/CatalogUtils.cs

@@ -5,17 +5,23 @@
 namespace Microsoft.ML.Data
 {
     /// <summary>
-    /// Set of extension methods to extract <see cref="IHostEnvironment"/> from various catalog classes.
+    /// Convenience method to more easily extract an <see cref="IHostEnvironment"/> from an <see cref="IInternalCatalog"/>
+    /// implementor without requiring an explicit cast.
     /// </summary>
     [BestFriend]
     internal static class CatalogUtils
     {
-        public static IHostEnvironment GetEnvironment(this TransformsCatalog catalog) => Contracts.CheckRef(catalog, nameof(catalog)).Environment;
-        public static IHostEnvironment GetEnvironment(this TransformsCatalog.SubCatalogBase subCatalog) => Contracts.CheckRef(subCatalog, nameof(subCatalog)).Environment;
-        public static IHostEnvironment GetEnvironment(this ModelOperationsCatalog catalog) => Contracts.CheckRef(catalog, nameof(catalog)).Environment;
-        public static IHostEnvironment GetEnvironment(this ModelOperationsCatalog.ExplainabilityTransforms catalog) => Contracts.CheckRef(catalog, nameof(catalog)).Environment;
-        public static IHostEnvironment GetEnvironment(this DataOperationsCatalog catalog) => Contracts.CheckRef(catalog, nameof(catalog)).Environment;
-        public static IHostEnvironment GetEnvironment(TrainCatalogBase.CatalogInstantiatorBase obj) => Contracts.CheckRef(obj, nameof(obj)).Owner.Environment;
-        public static IHostEnvironment GetEnvironment(TrainCatalogBase catalog) => Contracts.CheckRef(catalog, nameof(catalog)).Environment;
+        public static IHostEnvironment GetEnvironment(this IInternalCatalog catalog) => Contracts.CheckRef(catalog, nameof(catalog)).Environment;
+    }
+
+    /// <summary>
+    /// An internal interface for the benefit of those <see cref="IHostEnvironment"/>-bearing objects accessible through
+    /// <see cref="MLContext"/>. Because this is meant to consumed by component authors implementations of this interface
+    /// should be explicit.
+    /// </summary>
+    [BestFriend]
+    internal interface IInternalCatalog
+    {
+        IHostEnvironment Environment { get; }
     }
 }