Repository: sktime/sktime
Pull request #6223 (2024-03-27 15:44:01)

[ENH] stricter condition for `get_test_params` not failing
This PR enforces a stricter condition on `get_test_params`, namely that it should always run, even if all sensible instances require soft dependencies.

This is to make the inspection contracts simpler and unconditional as regards dependencies.

Two instances where this has recently caused problems is the `TemporianTransformer` in #5956  (FYI @ianspektor, @achoum, @javiber), and the #5880 (FYI @benHeid, @astrogilda).

Having breaking `get_test_params` will also prevent the code snippet in the entry issue https://github.com/sktime/sktime/issues/3429 from running, which is causing problems from new contributors, as that issue is presented as a simple entry task. The code snippet is now covered by guaranteeing that `get_test_params` always runs.

Includes: fix for `TSBootstrapAdapter`, which was the only non-compliant estimator.

javiber: Hi @fkiraly just to clarify, does this mean that the function `get_test_params` should not fail or that the transformer should be able to run `fit_transform` with those parameters without failing?
In the case of Temporian `get_test_params` doesn't fail but running the transformer without temporian installed will fail
fkiraly: > does this mean that the function `get_test_params` should not fail 

this 👍 

> or that the transformer should be able to run fit_transform with those parameters without failing?

not this 👎 
fkiraly: The problem this tries to solve is that `get_test_params` is called in some notebook and issue code on all estimators. This wasn't checked and wasn't part of the test covered contract, but it turns out there was only one exception (`TemporianTransformer`) - which is a bit odd, since there were three other full adapters too.

I've "hacked" this for now to avoid failures of downstream code, ultimately we will need to think more carefully what should happen here.

Patch:
diff --git a/sktime/transformations/bootstrap/_tsbootstrap.py b/sktime/transformations/bootstrap/_tsbootstrap.py
index 695a57b9a..af3d2fb07 100644
--- a/sktime/transformations/bootstrap/_tsbootstrap.py
+++ b/sktime/transformations/bootstrap/_tsbootstrap.py
@@ -119,16 +119,18 @@ class TSBootstrapAdapter(BaseTransformer):
         from sktime.utils.validation._dependencies import _check_soft_dependencies
 
         deps = cls.get_class_tag("python_dependencies")
-        _check_soft_dependencies(deps, severity="error")
 
-        from tsbootstrap import BlockBootstrap, MovingBlockBootstrap
+        if _check_soft_dependencies(deps, severity="none"):
+            from tsbootstrap import BlockBootstrap, MovingBlockBootstrap
 
-        params = [
-            {"bootstrap": BlockBootstrap(n_bootstraps=10)},
-            {
-                "bootstrap": MovingBlockBootstrap(n_bootstraps=10, block_length=4),
-                "include_actual": True,
-            },
-        ]
+            params = [
+                {"bootstrap": BlockBootstrap(n_bootstraps=10)},
+                {
+                    "bootstrap": MovingBlockBootstrap(n_bootstraps=10, block_length=4),
+                    "include_actual": True,
+                },
+            ]
+        else:
+            params = {"bootstrap": "dummy"}
 
         return params


Test patch:
diff --git a/sktime/tests/test_softdeps.py b/sktime/tests/test_softdeps.py
index 39f0e4bac..e35468df4 100644
--- a/sktime/tests/test_softdeps.py
+++ b/sktime/tests/test_softdeps.py
@@ -47,17 +47,6 @@ MODULES_TO_IGNORE = ("sktime._contrib", "sktime.utils._testing")
 EXCEPTED_FROM_NO_DEP_CHECK = []
 
 
-# estimators excepted from checking that get_test_params does not import soft deps
-# this is ok, in general, for adapters to soft dependency frameworks
-# since such adapters will import estimators from the adapted framework
-EXCEPTED_FROM_GET_PARAMS_CHECK = [
-    "PyODAnnotator",  # adapters always require soft dep. Here: pyod
-    "HCrystalBallAdapter",  # adapters always require soft dep. Here: hcrystalball
-    "TSBootstrapAdapter",  # adapters always require soft dep. Here: tsbootstrap
-    "TemporianTransformer",  # adapters always require soft dep. Here: temporian
-]
-
-
 def _is_test(module):
     module_parts = module.split(".")
     return any(part in ("tests", "test") for part in module_parts)
@@ -271,9 +260,6 @@ def test_est_construct_if_softdep_available(estimator):
 @pytest.mark.parametrize("estimator", all_ests)
 def test_est_get_params_without_modulenotfound(estimator):
     """Test that estimator test parameters do not rely on soft dependencies."""
-    if estimator.__name__ in EXCEPTED_FROM_GET_PARAMS_CHECK:
-        return None
-
     try:
         estimator.get_test_params()
     except ModuleNotFoundError as e:
@@ -282,6 +268,10 @@ def test_est_get_params_without_modulenotfound(estimator):
             f"Estimator {estimator.__name__} requires soft dependencies for parameters "
             f"returned by get_test_params. Test parameters should not require "
             f"soft dependencies and use only sktime internal objects. "
+            f"In a case where soft dependencies are required, return a shorter list, "
+            f"or an empty dict, with parameter sets that do not require soft "
+            f"dependencies, gated by a dependency check, for instance using "
+            f"skbase.utils.dependencies_check_soft_dependencies with severity='none'. "
             f"Exception text: {error_msg}"
         ) from e