From a46db4882de765195d2d8201b0366363cbb636fd Mon Sep 17 00:00:00 2001 From: adrinjalali Date: Mon, 21 Nov 2022 13:57:27 +0100 Subject: [PATCH 1/5] MNT error in init if given file is empty --- skops/hub_utils/_hf_hub.py | 3 +-- skops/hub_utils/tests/test_hf_hub.py | 11 ++++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/skops/hub_utils/_hf_hub.py b/skops/hub_utils/_hf_hub.py index 07c6c323..2c2fd323 100644 --- a/skops/hub_utils/_hf_hub.py +++ b/skops/hub_utils/_hf_hub.py @@ -8,7 +8,6 @@ import json import os import shutil -import warnings from pathlib import Path from typing import Any, List, MutableMapping, Optional, Union @@ -235,7 +234,7 @@ def _check_model_file(path: str | Path) -> Path: raise OSError(f"Model file '{path}' does not exist.") if os.path.getsize(path) == 0: - warnings.warn(f"Model file '{path}' is empty.") + raise RuntimeError(f"Model file '{path}' is empty.") return Path(path) diff --git a/skops/hub_utils/tests/test_hf_hub.py b/skops/hub_utils/tests/test_hf_hub.py index cdd5f17e..0d09f8a0 100644 --- a/skops/hub_utils/tests/test_hf_hub.py +++ b/skops/hub_utils/tests/test_hf_hub.py @@ -307,17 +307,16 @@ def test_model_file_does_not_exist_raises(repo_path, config_json): path_unlink(model_path, missing_ok=True) -def test_init_empty_model_file_warns(repo_path, config_json): +def test_init_empty_model_file_errors(repo_path, config_json): # when model file is empty, warn users - model_path = repo_path / "foobar.pickle" - with open(model_path, "wb"): - pass + model_path = Path(repo_path / "foobar.pickle") + model_path.touch() dir_path = tempfile.mkdtemp() shutil.rmtree(dir_path) version = metadata.version("scikit-learn") - with pytest.warns() as rec: + with pytest.raises(RuntimeError, match=f"Model file '{model_path}' is empty."): init( model=model_path, requirements=[f'scikit-learn="{version}"'], @@ -325,8 +324,6 @@ def test_init_empty_model_file_warns(repo_path, config_json): task="tabular-classification", data=iris.data, ) - assert len(rec) == 1 - assert rec[0].message.args[0] == f"Model file '{model_path}' is empty." path_unlink(model_path, missing_ok=True) From 7cf3177397c31ff4217c94f2478ea5a86c323025 Mon Sep 17 00:00:00 2001 From: adrinjalali Date: Mon, 21 Nov 2022 13:59:43 +0100 Subject: [PATCH 2/5] add changelog --- docs/changes.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changes.rst b/docs/changes.rst index ed62c90e..7236f3fe 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -22,6 +22,8 @@ v0.3 receive feedback from users. - Fix a bug that resulted in markdown tables being rendered incorrectly if entries contained line breaks. :pr:`156` by `Benjamin Bossan`_. +- Raise an error instead of warning the user if a given model file is empty. + :pr:`214` by `Adrin Jalali`_. - Use ``huggingface_hub`` v0.10.1 for model cards, drop ``modelcards`` dependency. :pr:`162` by `Benjamin Bossan`_. - Add source links to API documentation. :pr:`172` by `Ayyuce Demirbas`_. From a660c72ab1b991171d57943911aa4643fbf52409 Mon Sep 17 00:00:00 2001 From: adrinjalali Date: Mon, 21 Nov 2022 14:33:16 +0100 Subject: [PATCH 3/5] escape message --- skops/hub_utils/_hf_hub.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/skops/hub_utils/_hf_hub.py b/skops/hub_utils/_hf_hub.py index 2c2fd323..1795c312 100644 --- a/skops/hub_utils/_hf_hub.py +++ b/skops/hub_utils/_hf_hub.py @@ -7,6 +7,7 @@ import collections import json import os +import re import shutil from pathlib import Path from typing import Any, List, MutableMapping, Optional, Union @@ -234,7 +235,7 @@ def _check_model_file(path: str | Path) -> Path: raise OSError(f"Model file '{path}' does not exist.") if os.path.getsize(path) == 0: - raise RuntimeError(f"Model file '{path}' is empty.") + raise RuntimeError(re.escape(f"Model file '{path}' is empty.")) return Path(path) From 7e98e7b8a4341fe541f998e61b36b688fda42a12 Mon Sep 17 00:00:00 2001 From: adrinjalali Date: Tue, 22 Nov 2022 15:43:12 +0100 Subject: [PATCH 4/5] escape error message --- skops/hub_utils/tests/test_hf_hub.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/skops/hub_utils/tests/test_hf_hub.py b/skops/hub_utils/tests/test_hf_hub.py index 0d09f8a0..5aa229d1 100644 --- a/skops/hub_utils/tests/test_hf_hub.py +++ b/skops/hub_utils/tests/test_hf_hub.py @@ -316,7 +316,9 @@ def test_init_empty_model_file_errors(repo_path, config_json): shutil.rmtree(dir_path) version = metadata.version("scikit-learn") - with pytest.raises(RuntimeError, match=f"Model file '{model_path}' is empty."): + with pytest.raises( + RuntimeError, match=re.escape(f"Model file '{model_path}' is empty.") + ): init( model=model_path, requirements=[f'scikit-learn="{version}"'], From 3c226bacb151dab1cb5c06cf429c8c22304a9ab4 Mon Sep 17 00:00:00 2001 From: adrinjalali Date: Tue, 22 Nov 2022 15:53:43 +0100 Subject: [PATCH 5/5] don't escape the raised message --- skops/hub_utils/_hf_hub.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/skops/hub_utils/_hf_hub.py b/skops/hub_utils/_hf_hub.py index 1795c312..2c2fd323 100644 --- a/skops/hub_utils/_hf_hub.py +++ b/skops/hub_utils/_hf_hub.py @@ -7,7 +7,6 @@ import collections import json import os -import re import shutil from pathlib import Path from typing import Any, List, MutableMapping, Optional, Union @@ -235,7 +234,7 @@ def _check_model_file(path: str | Path) -> Path: raise OSError(f"Model file '{path}' does not exist.") if os.path.getsize(path) == 0: - raise RuntimeError(re.escape(f"Model file '{path}' is empty.")) + raise RuntimeError(f"Model file '{path}' is empty.") return Path(path)