refactor again to use the manifest to get metatdata by default

Author: ablaom

Manifest.toml

@@ -15,13 +15,17 @@ Suppressor = "fd094767-a336-5f1f-9728-57cf17d0bbfb"
 Distributed = "1.11.0"
 InteractiveUtils = "1.11.0"
 MLJModelInterface = "1.11.1"
-OrderedCollections = "1.8.1"
+MLJModels = "0.17"
+OrderedCollections = "1.8"
 Pkg = "1.11.0"
+Random = "1.11.0"
 Suppressor = "0.2"
 julia = "1.10"
 
 [extras]
+MLJModels = "d491faf4-2d78-11e9-2867-c94bc002c0b7"
+Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [targets]
-test = ["Test",]
+test = ["Test", "MLJModels", "Random"]

Project.toml

@@ -18,13 +18,16 @@ MLJGLMInterface = "caf8df21-4939-456d-ac9c-5fefbfb04c0c"
 MLJIteration = "614be32b-d00c-4edb-bd02-1eb411ab5e55"
 MLJLIBSVMInterface = "61c7150f-6c77-4bb1-949c-13197eac2a52"
 MLJLinearModels = "6ee0df7b-362f-4a72-a706-9e79364fb692"
+MLJModelInterface = "e80e1ace-859a-464e-9ed9-23947d8ae3ea"
+MLJModelRegistry = "0a96183e-380b-4aa6-be10-c555140810f2"
 MLJModels = "d491faf4-2d78-11e9-2867-c94bc002c0b7"
 MLJMultivariateStatsInterface = "1b6a4a23-ba22-4f51-9698-8599985d3728"
 MLJNaiveBayesInterface = "33e4bacb-b9e2-458e-9a13-5d9a90b235fa"
 MLJScikitLearnInterface = "5ae90465-5518-4432-b9d2-8a1def2f0cab"
 MLJTSVDInterface = "7fa162e1-0e29-41ca-a6fa-c000ca4e7e7e"
 MLJTestInterface = "72560011-54dd-4dc2-94f3-c5de45b75ecd"
 MLJText = "5e27fcf9-6bac-46ba-8580-b5712f3d6387"
+MLJTransforms = "23777cdb-d90c-4eb0-a694-7c2b83d5c1d6"
 MLJTuning = "03970b2e-30c4-11ea-3135-d1576263f10f"
 MLJXGBoostInterface = "54119dfa-1dab-4055-a167-80440f4f7a91"
 Maxnet = "81f79f80-22f2-4e41-ab86-00c11cf0f26f"

registry/Metadata.toml

@@ -111,36 +111,74 @@ end
 # # METHODS
 
 """
-    GenericRegistry.run([setup,] packages, program)
+    GenericRegistry.run([setup,] packages, program; environment=nothing)
 
-In a temporary Julia process, and using a temporary package environment, do this:
+Assuming a package `environment` path is specified, do the following in a new Julia
+process:
 
-1. Evaluate the `setup` expression (if present).
+1.Activate `environment`.
 
-2. Add the specified `packages` to the environment, and instantiate.
+2. Evaluate the `setup` expression, if specified.
+
+3. Instantiate the environment.
+
+4. `import` all packages specified in `packages`.
 
 3. Evaluate the `program` expression.
 
-The returned value is a `Future` object which must be `fetch`ed to get the actual
-evaluated expression. Shut the temporary process down by calling `close` on the `Future`.
+The returned value is a `Future` object which must be `fetch`ed to get the final evaluated
+expression. Shut the temporary process down by calling `GenericRegistry.close` on the
+`Future`.
+
+Step 3 might typically close by reversing any actions mutating the `environment`, but
+remember only the last evaluated expression is passed to the `Future`.
+
+If `environment` is unspecified, then a fresh temporary environment is activated, and the
+packages listed in `packages` are manually added between Steps 2 and 3 above.
 
 """
-function run(setup, pkgs, program)
+function run(setup, pkgs, program; environment=nothing)
     pkgs isa Vector || (pkgs = [pkgs,])
-    additions = [:(Pkg.add($pkg)) for pkg in pkgs]
     imports =  [:(import $(Symbol(pkg))) for pkg in pkgs]
-    program = quote
+    ex = quote
         using Pkg
-        Pkg.activate(temp=true)
-        $(additions...)
-        $setup
-        Pkg.instantiate()
-        $(imports...)
-        $program
     end
-    return run_in_temporary_process(program)
+    if isnothing(environment)
+        push!(
+            ex.args,
+            quote
+                Pkg.activate(temp=true)
+            end,
+        )
+    else
+        push!(
+            ex.args,
+            quote
+                Pkg.activate($environment)
+            end,
+        )
+    end
+    push!(ex.args, quote $setup end)
+    if isnothing(environment)
+        additions = [:(Pkg.add($pkg)) for pkg in pkgs]
+        push!(
+            ex.args,
+            quote
+                $(additions...)
+            end,
+        )
+    end
+    push!(
+        ex.args,
+        quote
+            Pkg.instantiate()
+            $(imports...)
+            $program
+        end,
+    )
+    return run_in_temporary_process(ex)
 end
-run(pkgs, program) = run(:(), pkgs, program)
+run(pkgs, program; kwargs...) = run(:(), pkgs, program; kwargs...)
 
 """
     GenericRegistry.close(future)

registry/Project.toml

@@ -15,19 +15,18 @@ Module providing methods for managing the MLJ Model Registry. To modify the regi
 
 - To add or update the metadata associated with a package, run [`update(pkg)`](@ref).
 
-- Alternatively, to update the metadata for *all* packages in the registry (optional but
-  recommended), run [`update()`](@ref).
+- Assuming this is successful, update the metadata for *all* packages in the registry
+  by running [`update()`](@ref).
 
 - When satisfied, commit your changes to the clone and make a pull request to the master
   MLJModelRegistry.jl repository.
 
-!!! important
+!!! note
 
     Removing a package from the "/registry/" enviroment does not remove its metadata from
-    the Model Registry (i.e., from "/registry/Metatdata.toml"). Unless you later call
-    `update()` to update all package metadata (slow), you must call
-    [`MLJModelRegistry.gc()`](@ref) to specifically remove metadata for all orphaned
-    packages (fast).
+    the Model Registry (i.e., from "/registry/Metatdata.toml"). However if you call
+    `update()` to update all package metadata (or call [`MLJModelRegistry.gc()`](@ref))
+    the metadata for all orphaned packages is removed.
 
 # Protocol for adding new packages to the registry environment
 
@@ -70,8 +69,14 @@ struct Quiet end
 # The MLJ Model Registry is a special case of a "generic model registry", as described in
 # this file, defining the `GenericRegistry` module (which has methods, no types):
 include("GenericRegistry.jl")
+
+# method to apply smoke tests to trait values for a model type:
 include("check_traits.jl")
+
+# methods called on remote processes to help extract metadata for a package's models:
 include("remote_methods.jl")
+
+# top-level methods for registry management:
 include("methods.jl")
 
 export update

src/GenericRegistry.jl

@@ -1,28 +1,42 @@
-# TODO: This method should live at MLJModelInterface !
+# # LOGGING
+
+err_bad_trait(M) = ErrorException("$M has a bad trait declaration. ")
+function err_bad_trait(M, message)
+    @error message
+    throw(err_bad_trait(M))
+end
+
+# # HELPERS
 
 ismissing_or_isa(x, T) = ismissing(x) || x isa T
 
+
+# # METHOD TO CHECK TRAITS
+
 function check_traits(M)
-    message = "$M has a bad trait declaration.\n"
-    ismissing_or_isa(MLJModelInterface.is_pure_julia(M), Bool) ||
-        error(message*"`is_pure_julia` must return true or false")
-    ismissing_or_isa(MLJModelInterface.supports_weights(M), Bool) ||
-        error(message*"`supports_weights` must return `true`, "*
-        "`false` or `missing`. ")
-    ismissing_or_isa(MLJModelInterface.supports_class_weights(M), Bool) ||
-        error(message*"`supports_class_weights` must return `true`, "*
+    if !ismissing_or_isa(MLJModelInterface.is_pure_julia(M), Bool)
+        err_bad_trait(M, "`is_pure_julia` must return true or false")
+    end
+    if !ismissing_or_isa(MLJModelInterface.supports_weights(M), Bool)
+        err_bad_trait(M, "`supports_weights` must return `true`, "*
+            "`false` or `missing`. ")
+    end
+    if !ismissing_or_isa(MLJModelInterface.supports_class_weights(M), Bool)
+        err_bad_trait(M, "`supports_class_weights` must return `true`, "*
             "`false` or `missing`. ")
-    MLJModelInterface.is_wrapper(M) isa Bool ||
-        error(message*"`is_wrapper` must return `true` or `false`. ")
+    end
+    if !(MLJModelInterface.is_wrapper(M) isa Bool)
+        err_bad_trait(M, "`is_wrapper` must return `true` or `false`. ")
+    end
     load_path = MLJModelInterface.load_path(M)
     load_path isa String ||
-        error(message*"`load_path` must return a `String`. ")
+        err_bad_trait(M, "`load_path` must return a `String`. ")
     contains(load_path, "unknown") &&
-        error(message*"`load_path` return value contains string \"unknown\". ")
+        err_bad_trait(M, "`load_path` return value contains string \"unknown\". ")
     pkg = MLJModelInterface.package_name(M)
-    pkg isa String || error(message*"`package_name` must return a `String`. ")
+    pkg isa String || err_bad_trait(M, "`package_name` must return a `String`. ")
     api_pkg = split(load_path, '.') |> first
-    pkg == "unknown" && error(message*"`package_name` returns \"unknown\". ")
+    pkg == "unknown" && err_bad_trait(M, "`package_name` returns \"unknown\". ")
     load_path_ex = Meta.parse(load_path)
     api_pkg_ex = Symbol(api_pkg)
     import_ex = :(import $api_pkg_ex)
@@ -33,7 +47,7 @@ function check_traits(M)
     try
         Main.eval(program_to_test_load_path)
     catch excptn
-        error(message*"Cannot import value of `load_path` (parsed as expression). ")
+        err_bad_trait(M, "Cannot import value of `load_path` (parsed as expression). ")
         rethrow(excptn)
     end
 end