diff --git a/CHANGES.rst b/CHANGES.rst index ba350c85d..85684234d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -33,6 +33,8 @@ New features Changes ------- +- The :meth: `DataOp.skb.full_report` method now deletes reports created with + ``output_dir=None`` after 7 days. :pr:`1657` by :user: `Simon Dierickx `. - The :func: `tabular_pipeline` uses a :class:`SquashingScaler` instead of a :class:`StandardScaler` for centering and scaling numerical features when linear models are used. diff --git a/skrub/_data_ops/_inspection.py b/skrub/_data_ops/_inspection.py index a3b6ad571..6861e13ef 100644 --- a/skrub/_data_ops/_inspection.py +++ b/skrub/_data_ops/_inspection.py @@ -80,11 +80,9 @@ def node_report(data_op, mode="preview", environment=None, **report_kwargs): def _get_output_dir(output_dir, overwrite): now = datetime.datetime.now().strftime("%Y-%m-%dT%H%M%S") if output_dir is None: - output_dir = ( - datasets.get_data_dir() - / "execution_reports" - / f"full_data_op_report_{now}_{random_string()}" - ) + output_base_dir = datasets.get_data_dir() / "execution_reports" + output_dir = output_base_dir / f"full_data_op_report_{now}_{random_string()}" + _utils.prune_directory(output_base_dir) else: output_dir = Path(output_dir).expanduser().resolve() if output_dir.exists(): diff --git a/skrub/_data_ops/_skrub_namespace.py b/skrub/_data_ops/_skrub_namespace.py index d4b0d0ab5..9d9b6fa51 100644 --- a/skrub/_data_ops/_skrub_namespace.py +++ b/skrub/_data_ops/_skrub_namespace.py @@ -1473,7 +1473,15 @@ def full_report( This creates a report showing the computation graph, and for each intermediate computation, some information (the line of code where it was defined, the time it took to run, and more) and a display of the - intermediate result (or error). + intermediate result (or error). By default, the report is stored in + a timestamped subdirectory of the skrub data folder. + + .. note:: + When this function is invoked reports starting with ``full_data_op_report_`` + that are stored in the skrub data folder are automatically deleted after 7 days. + This is to avoid accumulating too many reports over time. If you want to keep + specific reports, please specify an output directory. + Parameters ---------- @@ -1487,7 +1495,9 @@ def full_report( output_dir : str or Path or None (default=None) Directory where to store the report. If ``None``, a timestamped - subdirectory will be created in the skrub data directory. + subdirectory will be created in the skrub data directory. Note + that the reports created with ``output_dir=None`` are automatically + deleted after 7 days. overwrite : bool (default=False) What to do if the output directory already exists. If diff --git a/skrub/_data_ops/_utils.py b/skrub/_data_ops/_utils.py index 95a239d22..96e2a2e24 100644 --- a/skrub/_data_ops/_utils.py +++ b/skrub/_data_ops/_utils.py @@ -1,5 +1,10 @@ import enum +import re +import shutil +import time import traceback +import warnings +from pathlib import Path from joblib.externals import cloudpickle @@ -62,3 +67,26 @@ def format_exception(e): def format_exception_only(e): """compatibility for python < 3.10""" return traceback.format_exception_only(type(e), e) + + +def prune_directory(path: str): + path = Path(path) + if not path.exists(): + return + time_threshold = time.time() - 7 * 24 * 3600 # 7 days ago + # pattern to match folder names like full_data_op_report_{datetime}_{randomstring} + pattern = re.compile(r"^full_data_op_report_\d{4}-\d{2}-\d{2}T\d{6}_[0-9a-f]{8}$") + for dir_path in path.iterdir(): + if ( + dir_path.is_dir() + and pattern.match(dir_path.name) + and dir_path.stat().st_mtime < time_threshold + ): + try: + shutil.rmtree(dir_path) + except Exception as e: + warnings.warn( + "Skrub wants to delete an old folder in the skrub data folder: " + f"Could not delete {dir_path}:\n" + + "".join(format_exception_only(e)) + ) diff --git a/skrub/_data_ops/tests/__init__.py b/skrub/_data_ops/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/skrub/_data_ops/tests/test_utils.py b/skrub/_data_ops/tests/test_utils.py new file mode 100644 index 000000000..db305f04a --- /dev/null +++ b/skrub/_data_ops/tests/test_utils.py @@ -0,0 +1,81 @@ +import datetime +import os +import shutil +import tempfile +import time +from pathlib import Path + +import pytest + +from skrub._data_ops import _utils +from skrub._utils import random_string + + +@pytest.fixture +def tmp_dir(): + with tempfile.TemporaryDirectory() as tmpdir: + yield Path(tmpdir) + + +# Fixture to create directories with optional old timestamp +@pytest.fixture +def create_dir(tmp_dir): + def _create(name=None, days_old=None): + if name is None: + now = datetime.datetime.now().strftime("%Y-%m-%dT%H%M%S") + name = f"full_data_op_report_{now}_{random_string()}" + path = tmp_dir / name + path.mkdir() + if days_old is not None: + old_time = time.time() - days_old * 24 * 3600 + # setting access and modified times to old_time + os.utime(path, (old_time, old_time)) + return path + + return _create + + +def test_prune_directory_with_standard_name_dirs(tmp_dir, create_dir): + # Making an directory older than 7 days that should be pruned + # and one recent directory that should not be pruned + create_dir(name=None, days_old=None) + create_dir(name=None, days_old=8) + + assert len(list(tmp_dir.iterdir())) == 2 + + _utils.prune_directory(tmp_dir) + + remaining_items = list(tmp_dir.iterdir()) + assert len(remaining_items) == 1 + + +def test_prune_directory_with_nonstandard_name_dirs(tmp_dir, create_dir): + # Making an directory older than 7 days with a non-matching name + # so it should not be pruned + create_dir("other_report", days_old=8) + + assert len(list(tmp_dir.iterdir())) == 1 + + _utils.prune_directory(tmp_dir) + + remaining_items = list(tmp_dir.iterdir()) + assert len(remaining_items) == 1 + assert remaining_items[0].name == "other_report" + + +def test_prune_directory_catch_exception(tmp_dir, create_dir, monkeypatch): + create_dir(name=None, days_old=8) + + def mock_rmtree(path, *args, **kwargs): + raise OSError("Cannot delete folder") + + monkeypatch.setattr(shutil, "rmtree", mock_rmtree) + + assert len(list(tmp_dir.iterdir())) == 1 + + with pytest.warns(UserWarning, match="Could not delete"): + _utils.prune_directory(tmp_dir) + + assert len(list(tmp_dir.iterdir())) == 1 + + monkeypatch.undo()