diff --git a/docs/changes.rst b/docs/changes.rst index 4db24835..f42e7b85 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -11,6 +11,8 @@ skops Changelog v0.17 ----- +- Fix ``skops update`` failing when a relative output path contains a + directory, including updates with ``--inplace``. - Loading a skops file now checks that each part of the file holds the kind of content its loader expects, for instance that the keyword arguments of a ``functools.partial`` are stored as a dict. A file that does not is refused diff --git a/skops/cli/_update.py b/skops/cli/_update.py index 8f07b1b9..8a801252 100644 --- a/skops/cli/_update.py +++ b/skops/cli/_update.py @@ -99,7 +99,9 @@ def _update_file( return None with tempfile.TemporaryDirectory() as tmp_dir: - tmp_output_file = Path(tmp_dir) / f"{output_file}.tmp" + # Only the file name is used, so a relative output path with directories + # (e.g. ``models/model.skops``) doesn't point into a missing directory. + tmp_output_file = Path(tmp_dir) / f"{Path(output_file).name}.tmp" dump(input_model, tmp_output_file) shutil.move(str(tmp_output_file), str(output_file)) logger.info(f"Updated skops file written to {output_file}") diff --git a/skops/cli/tests/test_update.py b/skops/cli/tests/test_update.py index 1f28af39..0768dfbb 100644 --- a/skops/cli/tests/test_update.py +++ b/skops/cli/tests/test_update.py @@ -85,6 +85,23 @@ def test_base_case_works_as_expected( mock_logger.error.assert_not_called() mock_logger.debug.assert_not_called() + @pytest.mark.parametrize("inplace", [True, False]) + def test_relative_path_with_directory( + self, tmp_path, monkeypatch, inplace: bool, dump_old_file, safe_obj + ): + # A relative output path with a directory component used to fail, since + # it was joined to the temporary directory as is. + monkeypatch.chdir(tmp_path.parent) + input_file = pathlib.Path(tmp_path.name) / f"{self.model_name}.skops" + output_file = None if inplace else input_file.with_name("updated.skops") + + _update._update_file( + input_file=input_file, output_file=output_file, inplace=inplace + ) + + updated_obj = load(output_file or input_file) + assert np.array_equal(updated_obj, safe_obj) + @mock.patch("skops.cli._update.dump") def test_only_diff( self, mock_dump: mock.MagicMock, skops_path: pathlib.Path, dump_old_file