[train+tune] Make some deprecations for 2.10 (ray-project#42812)

This PR hard-deprecates/removes APIs that were scheduled for removal: `TuneConfig(chdir_to_trial_dir)` and certain `SyncConfig` parameters. This PR also resolves some other miscellaneous TODOs.

---------

Signed-off-by: Justin Yu <justinvyu@anyscale.com>

doc/source/tune/examples/BUILD

@@ -21,9 +21,7 @@ py_test_run_all_notebooks(
         "pbt_ppo_example.ipynb",
         "tune-xgboost.ipynb",
         "pbt_transformers.ipynb",  # Transformers uses legacy Tune APIs.
-        # TODO(justinvyu): tune_sklearn uses removed experiment analysis method
-        "tune-sklearn.ipynb",
-    ],
+   ],
     tags = [
         "exclusive",
         "team:ml",

python/ray/train/_internal/data_config.py

@@ -6,9 +6,6 @@
 from ray.data import DataIterator, Dataset, ExecutionOptions, NodeIdStr
 from ray.data._internal.execution.interfaces.execution_options import ExecutionResources
 from ray.data.preprocessor import Preprocessor
-
-# TODO(justinvyu): Fix the circular import error
-from ray.train.constants import TRAIN_DATASET_KEY  # noqa
 from ray.util.annotations import DeveloperAPI, PublicAPI
 
 

python/ray/train/_internal/syncer.py

@@ -3,14 +3,12 @@
 import threading
 import time
 import traceback
-import warnings
 from dataclasses import dataclass
 from typing import Any, Callable, Dict, List, Optional, Tuple, Union
 
 from ray._private.thirdparty.tabulate.tabulate import tabulate
 from ray.train.constants import _DEPRECATED_VALUE
-from ray.util import log_once
-from ray.util.annotations import PublicAPI
+from ray.util.annotations import DeveloperAPI, PublicAPI
 from ray.widgets import Template
 
 logger = logging.getLogger(__name__)
@@ -70,24 +68,26 @@ class SyncConfig:
     syncer: Optional[Union[str, "Syncer"]] = _DEPRECATED_VALUE
     sync_on_checkpoint: bool = _DEPRECATED_VALUE
 
+    # TODO(justinvyu): [Deprecated] Remove in 2.11.
     def _deprecation_warning(self, attr_name: str, extra_msg: str):
         if getattr(self, attr_name) != _DEPRECATED_VALUE:
-            if log_once(f"sync_config_param_deprecation_{attr_name}"):
-                warnings.warn(
-                    f"`SyncConfig({attr_name})` is a deprecated configuration "
-                    "and will be ignored. Please remove it from your `SyncConfig`, "
-                    "as this will raise an error in a future version of Ray."
-                    f"{extra_msg}"
-                )
+            raise DeprecationWarning(
+                f"`SyncConfig({attr_name})` is a deprecated configuration "
+                "Please remove it from your `SyncConfig`. "
+                f"{extra_msg}"
+            )
 
     def __post_init__(self):
-        for (attr_name, extra_msg) in [
-            ("upload_dir", "\nPlease specify `train.RunConfig(storage_path)` instead."),
+        for attr_name, extra_msg in [
+            (
+                "upload_dir",
+                "\nPlease specify `ray.train.RunConfig(storage_path)` instead.",
+            ),
             (
                 "syncer",
                 "\nPlease implement custom syncing logic with a custom "
                 "`pyarrow.fs.FileSystem` instead, and pass it into "
-                "`train.RunConfig(storage_filesystem)`. "
+                "`ray.train.RunConfig(storage_filesystem)`. "
                 "See here: https://docs.ray.io/en/latest/train/user-guides/persistent-storage.html#custom-storage",  # noqa: E501
             ),
             ("sync_on_checkpoint", ""),
@@ -178,6 +178,7 @@ def wait(self, timeout: Optional[float] = None) -> Any:
         return result
 
 
+@DeveloperAPI
 class Syncer(abc.ABC):
     """Syncer class for synchronizing data between Ray nodes and remote (cloud) storage.
 

python/ray/tune/analysis/experiment_analysis.py

@@ -702,36 +702,3 @@ def make_stub_if_needed(trial: Trial) -> Trial:
 
         state["trials"] = [make_stub_if_needed(t) for t in state["trials"]]
         return state
-
-    # TODO(ml-team): [Deprecated] Remove in 2.8
-    @property
-    def best_logdir(self) -> str:
-        raise DeprecationWarning(
-            "`best_logdir` is deprecated. Use `best_trial.local_path` instead."
-        )
-
-    def get_best_logdir(
-        self,
-        metric: Optional[str] = None,
-        mode: Optional[str] = None,
-        scope: str = "last",
-    ) -> Optional[str]:
-        raise DeprecationWarning(
-            "`get_best_logdir` is deprecated. "
-            "Use `get_best_trial(...).local_path` instead."
-        )
-
-    def get_trial_checkpoints_paths(
-        self, trial: Trial, metric: Optional[str] = None
-    ) -> List[Tuple[str, Number]]:
-        raise DeprecationWarning(
-            "`get_trial_checkpoints_paths` is deprecated. "
-            "Use `get_best_checkpoint` or wrap this `ExperimentAnalysis` in a "
-            "`ResultGrid` and use `Result.best_checkpoints` instead."
-        )
-
-    def fetch_trial_dataframes(self) -> Dict[str, DataFrame]:
-        raise DeprecationWarning(
-            "`fetch_trial_dataframes` is deprecated. "
-            "Access the `trial_dataframes` property instead."
-        )

python/ray/tune/experiment/experiment.py

@@ -370,8 +370,8 @@ def local_path(self) -> Optional[str]:
     @property
     @Deprecated("Replaced by `local_path`")
     def local_dir(self):
-        # Deprecate: Raise in 2.5, Remove in 2.6
-        return self.local_path
+        # TODO(justinvyu): [Deprecated] Remove in 2.11.
+        raise DeprecationWarning("Use `local_path` instead of `local_dir`.")
 
     @property
     def remote_path(self) -> Optional[str]:
@@ -386,11 +386,10 @@ def checkpoint_config(self):
         return self.spec.get("checkpoint_config")
 
     @property
-    @Deprecated("Replaced by `checkpoint_dir`")
+    @Deprecated("Replaced by `local_path`")
     def checkpoint_dir(self):
-        # Deprecate: Raise in 2.5, Remove in 2.6
-        # Provided when initializing Experiment, if so, return directly.
-        return self.local_path
+        # TODO(justinvyu): [Deprecated] Remove in 2.11.
+        raise DeprecationWarning("Use `local_path` instead of `checkpoint_dir`.")
 
     @property
     def run_identifier(self):

python/ray/tune/experiment/trial.py

@@ -541,8 +541,8 @@ def local_experiment_path(self) -> str:
     @property
     @Deprecated("Replaced by `local_path`")
     def logdir(self) -> Optional[str]:
-        # Deprecate: Raise in 2.5, Remove in 2.6
-        return self.local_path
+        # TODO(justinvyu): [Deprecated] Remove in 2.11.
+        raise DeprecationWarning("Use `local_path` instead of `logdir`.")
 
     @property
     def local_path(self) -> Optional[str]:
@@ -627,8 +627,8 @@ def reset(self):
 
     @Deprecated("Replaced by `init_local_path()`")
     def init_logdir(self):
-        # Deprecate: Raise in 2.5, Remove in 2.6
-        self.init_local_path()
+        # TODO(justinvyu): [Deprecated] Remove in 2.11.
+        raise DeprecationWarning("Use `init_local_path` instead of `init_logdir`.")
 
     def init_local_path(self):
         """Init logdir."""

python/ray/tune/syncer.py

@@ -1,32 +1,16 @@
 import logging
-import warnings
 
 from ray.train._internal.syncer import SyncConfig as TrainSyncConfig
 from ray.util.annotations import Deprecated
-from ray.util.debug import log_once
 
 
 logger = logging.getLogger(__name__)
 
 
 @Deprecated
 class SyncConfig(TrainSyncConfig):
-    def __new__(cls: type, *args, **kwargs):
-        if log_once("sync_config_moved"):
-            warnings.warn(
-                "`tune.SyncConfig` has been moved to `train.SyncConfig`. "
-                "Please update your code to use `train.SyncConfig`, "
-                "as this will raise an error in a future Ray version.",
-                stacklevel=2,
-            )
-        return super(SyncConfig, cls).__new__(cls, *args, **kwargs)
-
-
-@Deprecated
-class Syncer:
     def __new__(cls: type, *args, **kwargs):
         raise DeprecationWarning(
-            "`tune.syncer.Syncer` has been deprecated. "
-            "Please implement custom syncing logic via a custom "
-            "`train.RunConfig(storage_filesystem)` instead."
+            "`ray.tune.SyncConfig` has been moved to `ray.train.SyncConfig`. "
+            "Please update your code to use `ray.train.SyncConfig`."
         )

python/ray/tune/tests/test_tuner.py

@@ -12,7 +12,6 @@
 import ray
 from ray import train, tune
 from ray.train import CheckpointConfig, RunConfig, ScalingConfig
-from ray.train.constants import RAY_CHDIR_TO_TRIAL_DIR
 from ray.train.examples.pytorch.torch_linear_example import (
     train_func as linear_train_func,
 )
@@ -420,11 +419,8 @@ def train_func(config):
 
 def test_tuner_no_chdir_to_trial_dir_deprecated(shutdown_only, chdir_tmpdir):
     """Test the deprecated `chdir_to_trial_dir` config."""
-    _test_no_chdir("tuner", {}, use_deprecated_config=True)
-
-    # The deprecated config will fallback ot setting the environment variable.
-    # Reset it for the following tests
-    os.environ.pop(RAY_CHDIR_TO_TRIAL_DIR, None)
+    with pytest.raises(DeprecationWarning):
+        _test_no_chdir("tuner", {}, use_deprecated_config=True)
 
 
 @pytest.mark.parametrize("runtime_env", [{}, {"working_dir": "."}])

python/ray/tune/tune.py

@@ -396,14 +396,7 @@ def run(
             unique identifier (such as `Trial.trial_id`) is used in each trial's
             directory name. Otherwise, trials could overwrite artifacts and checkpoints
             of other trials. The return value cannot be a path.
-        chdir_to_trial_dir: Deprecated. Use `RAY_CHDIR_TO_TRIAL_DIR=0` instead.
-            Whether to change the working directory of each worker
-            to its corresponding trial directory. Defaults to `True` to prevent
-            contention between workers saving trial-level outputs.
-            If set to `False`, files are accessible with paths relative to the
-            original working directory. However, all workers on the same node now
-            share the same working directory, so be sure to use
-            `ray.train.get_context().get_trial_dir()` as the path to save any outputs.
+        chdir_to_trial_dir: Deprecated. Set the `RAY_CHDIR_TO_TRIAL_DIR` env var instead
         sync_config: Configuration object for syncing. See train.SyncConfig.
         export_formats: List of formats that exported at the end of
             the experiment. Default is None.
@@ -666,16 +659,15 @@ class and registered trainables.
         )
         checkpoint_config.checkpoint_at_end = checkpoint_at_end
 
+    # TODO(justinvyu): [Deprecated] Remove in 2.11.
     if chdir_to_trial_dir != _DEPRECATED_VALUE:
-        warnings.warn(
-            "`chdir_to_trial_dir` is deprecated and will be removed. "
+        raise DeprecationWarning(
+            "`chdir_to_trial_dir` is deprecated. "
             f"Use the {RAY_CHDIR_TO_TRIAL_DIR} environment variable instead. "
             "Set it to 0 to disable the default behavior of changing the "
             "working directory.",
             DeprecationWarning,
         )
-        if chdir_to_trial_dir is False:
-            os.environ[RAY_CHDIR_TO_TRIAL_DIR] = "0"
 
     if num_samples == -1:
         num_samples = sys.maxsize

python/ray/tune/tune_config.py

@@ -58,15 +58,7 @@ class TuneConfig:
             directory name. Otherwise, trials could overwrite artifacts and checkpoints
             of other trials. The return value cannot be a path.
             NOTE: This API is in alpha and subject to change.
-        chdir_to_trial_dir: Deprecated. Use the `RAY_CHDIR_TO_TRIAL_DIR=0`
-            environment variable instead.
-            Whether to change the working directory of each worker
-            to its corresponding trial directory. Defaults to `True` to prevent
-            contention between workers saving trial-level outputs.
-            If set to `False`, files are accessible with paths relative to the
-            original working directory. However, all workers on the same node now
-            share the same working directory, so be sure to use
-            `ray.train.get_context().get_trial_dir()` as the path to save any outputs.
+        chdir_to_trial_dir: Deprecated. Set the `RAY_CHDIR_TO_TRIAL_DIR` env var instead
     """
 
     # Currently this is not at feature parity with `tune.run`, nor should it be.