From 22c4c714ed823e6ed8c81c6e08ab6802c0dd790c Mon Sep 17 00:00:00 2001 From: = Date: Tue, 6 Dec 2022 21:42:27 +0000 Subject: [PATCH 01/38] First draft with test outline --- skops/io/_cli.py | 76 ++++++++++++++++++++++++++++++++++ skops/io/tests/test_cli.py | 40 ++++++++++++++++++ skops/io/tests/test_persist.py | 7 +++- 3 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 skops/io/_cli.py create mode 100644 skops/io/tests/test_cli.py diff --git a/skops/io/_cli.py b/skops/io/_cli.py new file mode 100644 index 00000000..75d99d27 --- /dev/null +++ b/skops/io/_cli.py @@ -0,0 +1,76 @@ +from __future__ import annotations + +import argparse +import logging +import os +import pathlib +import pickle as pkl +from typing import Optional + +from skops.io import dumps, get_untrusted_types + + +def _convert( + input_file: os.PathLike, output_dir: pathlib.Path, is_trusted: bool = False +): + model_name = pathlib.Path(input_file).stem + + logging.info(f"Converting {model_name}") + + with open(input_file, "rb") as f: + obj = pkl.load(f) + skops_dump = dumps(obj) + + if not is_trusted: + untrusted_types = get_untrusted_types(data=skops_dump) + if len(untrusted_types) > 0: + # Print out untrusted types to command line + untrusted_str = "\n".join(untrusted_types) + print(untrusted_str) + logging.warning( + "Unsafe Types Detected!\n" + f"While converting {model_name}, " + "the following unsafe types were found: \n" + f"{untrusted_str}\n" + "To convert this anyway, add `-t` to this command." + ) + return + + logging.debug(f"No unsafe types found in {model_name}.") + + with open(output_dir / f"{model_name}.skops", "wb") as out_file: + print(f"Writing to {output_dir/model_name}.skops") + out_file.write(skops_dump) + + +def main_convert(command_line_args: Optional[list[str]] = None): + parser = argparse.ArgumentParser( + description="Convert input Pickle files to .skops files" + ) + parser.add_argument("input-files", type=argparse.FileType("r"), nargs="+") + parser.add_argument("-t", "--trusted", action="store_true", default=False) + parser.add_argument( + "-d", + "--debug", + help="Enable debug logging", + action="store_const", + dest="loglevel", + const=logging.DEBUG, + default=logging.WARNING, + ) + parser.add_argument( + "-v", + "--verbose", + help="Enable verbose logging", + action="store_const", + dest="loglevel", + const=logging.INFO, + ) + args = parser.parse_args() + + for input_file in args.input_files: + _convert( + input_file=input_file, + output_dir=pathlib.Path.cwd(), + is_trusted=args.trusted, + ) diff --git a/skops/io/tests/test_cli.py b/skops/io/tests/test_cli.py new file mode 100644 index 00000000..eee3ca2b --- /dev/null +++ b/skops/io/tests/test_cli.py @@ -0,0 +1,40 @@ +import pickle as pkl + +import numpy as np +import pytest + +from skops.io._cli import _convert + + +class MockUnsafeType: + def __init__(self): + pass + + +class TestConvert: + model_name = "a_model" + + @pytest.fixture + def pkl_path(self, tmp_path): + return tmp_path / f"{self.model_name}.pkl" + + @pytest.fixture + def skops_path(self, tmp_path): + return tmp_path / f"{self.model_name}.skops" + + @pytest.fixture + def write_safe_file(self, pkl_path): + obj = np.ndarray([1, 2, 3, 4]) + with open(pkl_path, "wb") as f: + pkl.dump(obj, f) + + @pytest.fixture + def write_unsafe_file(self, pkl_path): + obj = MockUnsafeType() + with open(pkl_path, "wb") as f: + pkl.dump(obj, f) + + def test_base_case_works_as_expected( + self, write_safe_file, pkl_path, skops_path, tmp_path + ): + _convert(pkl_path, tmp_path) diff --git a/skops/io/tests/test_persist.py b/skops/io/tests/test_persist.py index c38871c6..2160e259 100644 --- a/skops/io/tests/test_persist.py +++ b/skops/io/tests/test_persist.py @@ -419,7 +419,12 @@ def get_input(estimator): ) else: X, y = make_classification( - n_samples=N_SAMPLES, n_features=N_FEATURES, random_state=0 + n_samples=N_SAMPLES, + n_features=N_FEATURES, + random_state=0, + n_classes=3, + n_redundant=1, + n_informative=N_FEATURES - 1, ) y = _enforce_estimator_tags_y(estimator, y) tags = _safe_tags(estimator) From eacedf8b67fff6192d3b54b7a08e89730cc1a5f8 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 9 Dec 2022 21:53:23 +0000 Subject: [PATCH 02/38] Add in tests for both main and method --- skops/io/_cli.py | 38 ++++++++++-------- skops/io/tests/test_cli.py | 79 ++++++++++++++++++++++++++++++++++---- 2 files changed, 92 insertions(+), 25 deletions(-) diff --git a/skops/io/_cli.py b/skops/io/_cli.py index 75d99d27..65a2dee1 100644 --- a/skops/io/_cli.py +++ b/skops/io/_cli.py @@ -21,25 +21,29 @@ def _convert( obj = pkl.load(f) skops_dump = dumps(obj) - if not is_trusted: - untrusted_types = get_untrusted_types(data=skops_dump) - if len(untrusted_types) > 0: - # Print out untrusted types to command line - untrusted_str = "\n".join(untrusted_types) - print(untrusted_str) + untrusted_types = get_untrusted_types(data=skops_dump) + + if not untrusted_types: + logging.debug(f"No unsafe types found in {model_name}.") + else: + untrusted_str = "\n".join(untrusted_types) + + logging.warning( + "Unsafe Types Detected!\n" + f"While converting {model_name}, " + "the following unsafe types were found: \n" + f"{untrusted_str}\n" + ) + + if not is_trusted: logging.warning( - "Unsafe Types Detected!\n" - f"While converting {model_name}, " - "the following unsafe types were found: \n" - f"{untrusted_str}\n" + f"Model {model_name} will not be converted due to unsafe types.\n" "To convert this anyway, add `-t` to this command." ) return - logging.debug(f"No unsafe types found in {model_name}.") - with open(output_dir / f"{model_name}.skops", "wb") as out_file: - print(f"Writing to {output_dir/model_name}.skops") + logging.info(f"Writing to {output_dir/model_name}.skops") out_file.write(skops_dump) @@ -47,7 +51,7 @@ def main_convert(command_line_args: Optional[list[str]] = None): parser = argparse.ArgumentParser( description="Convert input Pickle files to .skops files" ) - parser.add_argument("input-files", type=argparse.FileType("r"), nargs="+") + parser.add_argument("inputs", nargs="+") parser.add_argument("-t", "--trusted", action="store_true", default=False) parser.add_argument( "-d", @@ -66,9 +70,9 @@ def main_convert(command_line_args: Optional[list[str]] = None): dest="loglevel", const=logging.INFO, ) - args = parser.parse_args() - - for input_file in args.input_files: + args = parser.parse_args(command_line_args) + print(args.inputs) + for input_file in args.inputs: _convert( input_file=input_file, output_dir=pathlib.Path.cwd(), diff --git a/skops/io/tests/test_cli.py b/skops/io/tests/test_cli.py index eee3ca2b..171696e6 100644 --- a/skops/io/tests/test_cli.py +++ b/skops/io/tests/test_cli.py @@ -1,8 +1,15 @@ +import logging +import os.path +import pathlib import pickle as pkl +from typing import Optional +from unittest import mock +from unittest.mock import call, patch import numpy as np import pytest +import skops from skops.io._cli import _convert @@ -12,7 +19,15 @@ def __init__(self): class TestConvert: - model_name = "a_model" + model_name = "some_model_name" + + @pytest.fixture + def safe_obj(self): + return np.ndarray([1, 2, 3, 4]) + + @pytest.fixture + def unsafe_obj(self): + return MockUnsafeType() @pytest.fixture def pkl_path(self, tmp_path): @@ -23,18 +38,66 @@ def skops_path(self, tmp_path): return tmp_path / f"{self.model_name}.skops" @pytest.fixture - def write_safe_file(self, pkl_path): - obj = np.ndarray([1, 2, 3, 4]) + def write_safe_file(self, pkl_path, safe_obj): with open(pkl_path, "wb") as f: - pkl.dump(obj, f) + pkl.dump(safe_obj, f) @pytest.fixture - def write_unsafe_file(self, pkl_path): - obj = MockUnsafeType() + def write_unsafe_file(self, pkl_path, unsafe_obj): with open(pkl_path, "wb") as f: - pkl.dump(obj, f) + pkl.dump(unsafe_obj, f) def test_base_case_works_as_expected( - self, write_safe_file, pkl_path, skops_path, tmp_path + self, pkl_path, tmp_path, skops_path, write_safe_file, safe_obj ): _convert(pkl_path, tmp_path) + persisted_obj = skops.io.load(skops_path) + assert np.array_equal(persisted_obj, safe_obj) + + def test_unsafe_case_works_as_expected( + self, pkl_path, tmp_path, skops_path, write_unsafe_file, caplog + ): + caplog.set_level(logging.WARNING) + _convert(pkl_path, tmp_path) + assert not os.path.isfile(skops_path) + + # check logging has warned that an unsafe type was found + assert MockUnsafeType.__name__ in caplog.text + + +class TestMainConvert: + @staticmethod + def assert_called_correctly( + mock_convert: mock.MagicMock, + paths: list, + output_dir: Optional[pathlib.Path] = pathlib.Path.cwd(), + trusted: Optional[bool] = False, + ): + assert mock_convert.call_count == len(paths) + + mock_convert.assert_has_calls( + [ + call(input_file=p, output_dir=output_dir, is_trusted=trusted) + for p in paths + ] + ) + + @patch("skops.io._cli._convert") + def test_base_works_as_expected(self, mock_convert: mock.MagicMock): + args = [ + "123.pkl", + "abc.pkl", + ] + + skops.io._cli.main_convert(command_line_args=args) + self.assert_called_correctly(mock_convert, args) + + @patch("skops.io._cli._convert") + @pytest.mark.parametrize("trusted_flag", ["-t", "--trusted"]) + def test_with_trusted_works_as_expected( + self, mock_convert: mock.MagicMock, trusted_flag + ): + paths = ["abc.123", "234.567", "d/object_1.pkl"] + args = paths + [trusted_flag] + skops.io._cli.main_convert(command_line_args=args) + self.assert_called_correctly(mock_convert, paths=paths, trusted=True) From 453728d3b6aaec553123bee13e131d40e9e74a1c Mon Sep 17 00:00:00 2001 From: = Date: Fri, 9 Dec 2022 22:07:15 +0000 Subject: [PATCH 03/38] Add entrypoint to setup.py --- setup.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index df25333b..17a13116 100644 --- a/setup.py +++ b/setup.py @@ -7,7 +7,7 @@ # This is a bit (!) hackish: we are setting a global variable so that the # main modelcard __init__ can detect if it is being loaded by the setup # routine, to avoid attempting to load components. -builtins.__SKOPS_SETUP__ = True +builtins.__SKOPS_SETUP__ = True # type: ignore import skops # noqa @@ -34,6 +34,14 @@ def setup_package(): + package_data = dict( + entry_points={ + "console_scripts": [ + "convert = skops.io._cli._convert:main_convert", + ], + } + ) + metadata = dict( name=DISTNAME, maintainer=MAINTAINER, @@ -73,7 +81,7 @@ def setup_package(): include_package_data=True, ) - setup(**metadata) + setup(**package_data, **metadata) if __name__ == "__main__": From da6da30b0bfdefea97f1f89e9f705042d6bede57 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 9 Dec 2022 22:22:53 +0000 Subject: [PATCH 04/38] Get entrypoint working --- setup.py | 2 +- skops/io/_cli.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 17a13116..9e94c001 100644 --- a/setup.py +++ b/setup.py @@ -37,7 +37,7 @@ def setup_package(): package_data = dict( entry_points={ "console_scripts": [ - "convert = skops.io._cli._convert:main_convert", + "skops-convert = skops.io._cli:main_convert", ], } ) diff --git a/skops/io/_cli.py b/skops/io/_cli.py index 65a2dee1..d66b777c 100644 --- a/skops/io/_cli.py +++ b/skops/io/_cli.py @@ -71,7 +71,6 @@ def main_convert(command_line_args: Optional[list[str]] = None): const=logging.INFO, ) args = parser.parse_args(command_line_args) - print(args.inputs) for input_file in args.inputs: _convert( input_file=input_file, From 5a9c0fc3db94b5f7628fc5632acce497a73eb857 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 9 Dec 2022 22:46:41 +0000 Subject: [PATCH 05/38] Undo change in _persist --- skops/io/tests/test_persist.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/skops/io/tests/test_persist.py b/skops/io/tests/test_persist.py index 2160e259..c38871c6 100644 --- a/skops/io/tests/test_persist.py +++ b/skops/io/tests/test_persist.py @@ -419,12 +419,7 @@ def get_input(estimator): ) else: X, y = make_classification( - n_samples=N_SAMPLES, - n_features=N_FEATURES, - random_state=0, - n_classes=3, - n_redundant=1, - n_informative=N_FEATURES - 1, + n_samples=N_SAMPLES, n_features=N_FEATURES, random_state=0 ) y = _enforce_estimator_tags_y(estimator, y) tags = _safe_tags(estimator) From b3a561eea8d668b4ee6f19bc335ef08dadd18383 Mon Sep 17 00:00:00 2001 From: = Date: Sun, 11 Dec 2022 13:24:57 +0000 Subject: [PATCH 06/38] Add output dir argument --- skops/io/_cli.py | 32 +++++++++++++++++++++++++++++--- skops/io/tests/test_cli.py | 21 ++++++++++++++++++++- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/skops/io/_cli.py b/skops/io/_cli.py index d66b777c..e2a0673e 100644 --- a/skops/io/_cli.py +++ b/skops/io/_cli.py @@ -51,8 +51,18 @@ def main_convert(command_line_args: Optional[list[str]] = None): parser = argparse.ArgumentParser( description="Convert input Pickle files to .skops files" ) - parser.add_argument("inputs", nargs="+") - parser.add_argument("-t", "--trusted", action="store_true", default=False) + parser.add_argument("inputs", nargs="+", help="Input files to convert.") + parser.add_argument( + "-t", + "--trusted", + help=( + "Automatically trust all files, " + "and convert all inputs, even if an " + "untrusted type is detected." + ), + action="store_true", + default=False, + ) parser.add_argument( "-d", "--debug", @@ -70,10 +80,26 @@ def main_convert(command_line_args: Optional[list[str]] = None): dest="loglevel", const=logging.INFO, ) + parser.add_argument( + "--output-dir", + help=( + "Specify a directory to save converted files to. " + "Default will save files to the current working directory." + ), + type=str, + default=None, + ) args = parser.parse_args(command_line_args) + + output_dir = args.output_dir + if not output_dir: + output_dir = pathlib.Path.cwd() + else: + output_dir = pathlib.Path(output_dir) + for input_file in args.inputs: _convert( input_file=input_file, - output_dir=pathlib.Path.cwd(), + output_dir=output_dir, is_trusted=args.trusted, ) diff --git a/skops/io/tests/test_cli.py b/skops/io/tests/test_cli.py index 171696e6..1fa565d9 100644 --- a/skops/io/tests/test_cli.py +++ b/skops/io/tests/test_cli.py @@ -73,6 +73,7 @@ def assert_called_correctly( output_dir: Optional[pathlib.Path] = pathlib.Path.cwd(), trusted: Optional[bool] = False, ): + breakpoint() assert mock_convert.call_count == len(paths) mock_convert.assert_has_calls( @@ -93,7 +94,7 @@ def test_base_works_as_expected(self, mock_convert: mock.MagicMock): self.assert_called_correctly(mock_convert, args) @patch("skops.io._cli._convert") - @pytest.mark.parametrize("trusted_flag", ["-t", "--trusted"]) + @pytest.mark.parametrize("trusted_flag", ["-t", "--trusted", None]) def test_with_trusted_works_as_expected( self, mock_convert: mock.MagicMock, trusted_flag ): @@ -101,3 +102,21 @@ def test_with_trusted_works_as_expected( args = paths + [trusted_flag] skops.io._cli.main_convert(command_line_args=args) self.assert_called_correctly(mock_convert, paths=paths, trusted=True) + + @patch("skops.io._cli._convert") + @pytest.mark.parametrize( + "output_dir, expected_dir", + [("a/b/c", pathlib.Path("a/b/c")), (None, pathlib.Path.cwd())], + ) + def test_with_output_dir_works_as_expected( + self, mock_convert: mock.MagicMock, output_dir, expected_dir + ): + paths = ["abc.123", "234.567", "d/object_1.pkl"] + + if output_dir is not None: + args = paths + ["--output-dir", output_dir] + else: + args = paths + + skops.io._cli.main_convert(command_line_args=args) + self.assert_called_correctly(mock_convert, paths=paths, output_dir=expected_dir) From 1d01ff5bae5c07d7294c808386ab6bba9a3a87c5 Mon Sep 17 00:00:00 2001 From: = Date: Sun, 11 Dec 2022 13:29:26 +0000 Subject: [PATCH 07/38] fix tests --- skops/io/tests/test_cli.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/skops/io/tests/test_cli.py b/skops/io/tests/test_cli.py index 1fa565d9..56a31305 100644 --- a/skops/io/tests/test_cli.py +++ b/skops/io/tests/test_cli.py @@ -73,7 +73,6 @@ def assert_called_correctly( output_dir: Optional[pathlib.Path] = pathlib.Path.cwd(), trusted: Optional[bool] = False, ): - breakpoint() assert mock_convert.call_count == len(paths) mock_convert.assert_has_calls( @@ -94,7 +93,7 @@ def test_base_works_as_expected(self, mock_convert: mock.MagicMock): self.assert_called_correctly(mock_convert, args) @patch("skops.io._cli._convert") - @pytest.mark.parametrize("trusted_flag", ["-t", "--trusted", None]) + @pytest.mark.parametrize("trusted_flag", ["-t", "--trusted"]) def test_with_trusted_works_as_expected( self, mock_convert: mock.MagicMock, trusted_flag ): From 91d1b0f9c57dfc8796de679e4264dd8a920dab26 Mon Sep 17 00:00:00 2001 From: = Date: Sun, 11 Dec 2022 13:41:42 +0000 Subject: [PATCH 08/38] adjust format of log message --- skops/io/_cli.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/skops/io/_cli.py b/skops/io/_cli.py index e2a0673e..225a1186 100644 --- a/skops/io/_cli.py +++ b/skops/io/_cli.py @@ -57,8 +57,8 @@ def main_convert(command_line_args: Optional[list[str]] = None): "--trusted", help=( "Automatically trust all files, " - "and convert all inputs, even if an " - "untrusted type is detected." + "and convert all inputs, " + "even if an untrusted type is detected." ), action="store_true", default=False, @@ -66,7 +66,7 @@ def main_convert(command_line_args: Optional[list[str]] = None): parser.add_argument( "-d", "--debug", - help="Enable debug logging", + help="Enable debug logging.", action="store_const", dest="loglevel", const=logging.DEBUG, @@ -75,7 +75,7 @@ def main_convert(command_line_args: Optional[list[str]] = None): parser.add_argument( "-v", "--verbose", - help="Enable verbose logging", + help="Enable verbose logging.", action="store_const", dest="loglevel", const=logging.INFO, @@ -97,6 +97,8 @@ def main_convert(command_line_args: Optional[list[str]] = None): else: output_dir = pathlib.Path(output_dir) + logging.basicConfig(format="%(message)s", level=args.loglevel) + for input_file in args.inputs: _convert( input_file=input_file, From 7af9c268113740ac1b74006614ae35157082f52e Mon Sep 17 00:00:00 2001 From: = Date: Sun, 11 Dec 2022 15:09:51 +0000 Subject: [PATCH 09/38] Update main entrypoint and parent structure --- setup.py | 2 +- skops/entrypoint.py | 21 +++++++++++++++++++++ skops/io/_cli.py | 12 +++++++++--- 3 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 skops/entrypoint.py diff --git a/setup.py b/setup.py index 9e94c001..32f9db1c 100644 --- a/setup.py +++ b/setup.py @@ -37,7 +37,7 @@ def setup_package(): package_data = dict( entry_points={ "console_scripts": [ - "skops-convert = skops.io._cli:main_convert", + "skops = skops.entrypoint:main_entrypoint", ], } ) diff --git a/skops/entrypoint.py b/skops/entrypoint.py new file mode 100644 index 00000000..5e751bbb --- /dev/null +++ b/skops/entrypoint.py @@ -0,0 +1,21 @@ +import argparse + +import skops.io._cli + + +def main_entrypoint(command_line_args=None): + """Main entrypoint for all command line Skops methods.""" + parser = argparse.ArgumentParser( + prog="Skops", + description="Main entrypoint for all command line Skops methods.", + add_help=False, + ) + + function_map = {"convert": skops.io._cli.main_convert} + + parser.add_argument( + "function", help="Function to call.", choices=function_map.keys() + ) + + arg, _ = parser.parse_known_args() + function_map.get(arg.function)(parent=parser) diff --git a/skops/io/_cli.py b/skops/io/_cli.py index 225a1186..45040262 100644 --- a/skops/io/_cli.py +++ b/skops/io/_cli.py @@ -43,14 +43,20 @@ def _convert( return with open(output_dir / f"{model_name}.skops", "wb") as out_file: - logging.info(f"Writing to {output_dir/model_name}.skops") + logging.info(f"Writing to {output_dir / model_name}.skops") out_file.write(skops_dump) -def main_convert(command_line_args: Optional[list[str]] = None): +def main_convert( + command_line_args: Optional[list[str]] = None, + parent: Optional[argparse.ArgumentParser] = None, +): + parents = [parent] if parent else [] + parser = argparse.ArgumentParser( - description="Convert input Pickle files to .skops files" + description="Convert input Pickle files to .skops files", parents=parents ) + parser.add_argument("inputs", nargs="+", help="Input files to convert.") parser.add_argument( "-t", From 8ad10cdfcc1c4f2e33da18889572e5cebbfc11fa Mon Sep 17 00:00:00 2001 From: = Date: Mon, 12 Dec 2022 21:06:29 +0000 Subject: [PATCH 10/38] Add test that covers main entrypoint --- setup.py | 2 +- skops/io/tests/test_cli.py | 9 ++++---- skops/utils/__init__.py | 0 skops/{ => utils}/entrypoint.py | 6 ++---- skops/utils/tests/test_entrypoint.py | 32 ++++++++++++++++++++++++++++ 5 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 skops/utils/__init__.py rename skops/{ => utils}/entrypoint.py (76%) create mode 100644 skops/utils/tests/test_entrypoint.py diff --git a/setup.py b/setup.py index 32f9db1c..efde11e1 100644 --- a/setup.py +++ b/setup.py @@ -37,7 +37,7 @@ def setup_package(): package_data = dict( entry_points={ "console_scripts": [ - "skops = skops.entrypoint:main_entrypoint", + "skops = skops.utils.entrypoint:main_entrypoint", ], } ) diff --git a/skops/io/tests/test_cli.py b/skops/io/tests/test_cli.py index 56a31305..7b186b6d 100644 --- a/skops/io/tests/test_cli.py +++ b/skops/io/tests/test_cli.py @@ -4,7 +4,6 @@ import pickle as pkl from typing import Optional from unittest import mock -from unittest.mock import call, patch import numpy as np import pytest @@ -77,12 +76,12 @@ def assert_called_correctly( mock_convert.assert_has_calls( [ - call(input_file=p, output_dir=output_dir, is_trusted=trusted) + mock.call(input_file=p, output_dir=output_dir, is_trusted=trusted) for p in paths ] ) - @patch("skops.io._cli._convert") + @mock.patch("skops.io._cli._convert") def test_base_works_as_expected(self, mock_convert: mock.MagicMock): args = [ "123.pkl", @@ -92,7 +91,7 @@ def test_base_works_as_expected(self, mock_convert: mock.MagicMock): skops.io._cli.main_convert(command_line_args=args) self.assert_called_correctly(mock_convert, args) - @patch("skops.io._cli._convert") + @mock.patch("skops.io._cli._convert") @pytest.mark.parametrize("trusted_flag", ["-t", "--trusted"]) def test_with_trusted_works_as_expected( self, mock_convert: mock.MagicMock, trusted_flag @@ -102,7 +101,7 @@ def test_with_trusted_works_as_expected( skops.io._cli.main_convert(command_line_args=args) self.assert_called_correctly(mock_convert, paths=paths, trusted=True) - @patch("skops.io._cli._convert") + @mock.patch("skops.io._cli._convert") @pytest.mark.parametrize( "output_dir, expected_dir", [("a/b/c", pathlib.Path("a/b/c")), (None, pathlib.Path.cwd())], diff --git a/skops/utils/__init__.py b/skops/utils/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/skops/entrypoint.py b/skops/utils/entrypoint.py similarity index 76% rename from skops/entrypoint.py rename to skops/utils/entrypoint.py index 5e751bbb..0933a804 100644 --- a/skops/entrypoint.py +++ b/skops/utils/entrypoint.py @@ -12,10 +12,8 @@ def main_entrypoint(command_line_args=None): ) function_map = {"convert": skops.io._cli.main_convert} - parser.add_argument( "function", help="Function to call.", choices=function_map.keys() ) - - arg, _ = parser.parse_known_args() - function_map.get(arg.function)(parent=parser) + args, _ = parser.parse_known_args(command_line_args) + function_map.get(args.function)(parent=parser, command_line_args=command_line_args) diff --git a/skops/utils/tests/test_entrypoint.py b/skops/utils/tests/test_entrypoint.py new file mode 100644 index 00000000..396636e9 --- /dev/null +++ b/skops/utils/tests/test_entrypoint.py @@ -0,0 +1,32 @@ +import pathlib +import sys +from unittest import mock + +import pytest + +from skops.utils.entrypoint import main_entrypoint + + +class TestEntrypoint: + """Integration tests that check that entrypoint calls pass through correctly. + + Full coverage of individual entrypoint calls should be done in their own classes. + """ + + @pytest.fixture(autouse=True) + def clear_argv(self): + # Required to clear argv in case Pytest is called on this specific function. + # Otherwise, clogs parser.parse_known_args() in argparse + sys.argv = [""] + + @mock.patch("skops.io._cli._convert") + def test_convert_works_as_expected(self, mocked_convert: mock.MagicMock): + args = ["convert", "abc.def", "-t"] + + main_entrypoint(args) + + mocked_convert.assert_called_once_with( + input_file="abc.def", + output_dir=pathlib.Path.cwd(), + is_trusted=True, + ) From 864d8ca6b570d21507d379893b473a8ca282121e Mon Sep 17 00:00:00 2001 From: Erin Aho Date: Thu, 15 Dec 2022 09:51:52 +0000 Subject: [PATCH 11/38] Update skops/io/_cli.py Co-authored-by: Adrin Jalali --- skops/io/_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skops/io/_cli.py b/skops/io/_cli.py index 45040262..673a1580 100644 --- a/skops/io/_cli.py +++ b/skops/io/_cli.py @@ -29,7 +29,7 @@ def _convert( untrusted_str = "\n".join(untrusted_types) logging.warning( - "Unsafe Types Detected!\n" + "Unknown Types Detected!\n" f"While converting {model_name}, " "the following unsafe types were found: \n" f"{untrusted_str}\n" From b39ec14fafcc4c4a53c5e7184f0da5754651675c Mon Sep 17 00:00:00 2001 From: = Date: Sat, 17 Dec 2022 18:30:19 +0000 Subject: [PATCH 12/38] WIP for merging from main --- skops/cli/__init__.py | 0 skops/{io/_cli.py => cli/_convert.py} | 1 + skops/{utils => cli}/entrypoint.py | 4 ++-- skops/cli/tests/__init__.py | 0 skops/{utils => cli}/tests/test_entrypoint.py | 2 +- skops/io/tests/test_cli.py | 10 +++++----- 6 files changed, 9 insertions(+), 8 deletions(-) create mode 100644 skops/cli/__init__.py rename skops/{io/_cli.py => cli/_convert.py} (97%) rename skops/{utils => cli}/entrypoint.py (86%) create mode 100644 skops/cli/tests/__init__.py rename skops/{utils => cli}/tests/test_entrypoint.py (94%) diff --git a/skops/cli/__init__.py b/skops/cli/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/skops/io/_cli.py b/skops/cli/_convert.py similarity index 97% rename from skops/io/_cli.py rename to skops/cli/_convert.py index 45040262..80a6129c 100644 --- a/skops/io/_cli.py +++ b/skops/cli/_convert.py @@ -13,6 +13,7 @@ def _convert( input_file: os.PathLike, output_dir: pathlib.Path, is_trusted: bool = False ): + """Function that is called by the ``skops convert`` entrypoint""" model_name = pathlib.Path(input_file).stem logging.info(f"Converting {model_name}") diff --git a/skops/utils/entrypoint.py b/skops/cli/entrypoint.py similarity index 86% rename from skops/utils/entrypoint.py rename to skops/cli/entrypoint.py index 0933a804..5e5c8335 100644 --- a/skops/utils/entrypoint.py +++ b/skops/cli/entrypoint.py @@ -1,6 +1,6 @@ import argparse -import skops.io._cli +import skops.cli._convert def main_entrypoint(command_line_args=None): @@ -11,7 +11,7 @@ def main_entrypoint(command_line_args=None): add_help=False, ) - function_map = {"convert": skops.io._cli.main_convert} + function_map = {"convert": skops.cli._cli.main_convert} parser.add_argument( "function", help="Function to call.", choices=function_map.keys() ) diff --git a/skops/cli/tests/__init__.py b/skops/cli/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/skops/utils/tests/test_entrypoint.py b/skops/cli/tests/test_entrypoint.py similarity index 94% rename from skops/utils/tests/test_entrypoint.py rename to skops/cli/tests/test_entrypoint.py index 396636e9..febf9164 100644 --- a/skops/utils/tests/test_entrypoint.py +++ b/skops/cli/tests/test_entrypoint.py @@ -4,7 +4,7 @@ import pytest -from skops.utils.entrypoint import main_entrypoint +from skops.cli.entrypoint import main_entrypoint class TestEntrypoint: diff --git a/skops/io/tests/test_cli.py b/skops/io/tests/test_cli.py index 7b186b6d..1a8ffaf2 100644 --- a/skops/io/tests/test_cli.py +++ b/skops/io/tests/test_cli.py @@ -9,7 +9,7 @@ import pytest import skops -from skops.io._cli import _convert +from skops.cli._convert import _convert class MockUnsafeType: @@ -81,14 +81,14 @@ def assert_called_correctly( ] ) - @mock.patch("skops.io._cli._convert") + @mock.patch("skops.cli._convert._convert") def test_base_works_as_expected(self, mock_convert: mock.MagicMock): args = [ "123.pkl", "abc.pkl", ] - skops.io._cli.main_convert(command_line_args=args) + skops.cli._convert.main_convert(command_line_args=args) self.assert_called_correctly(mock_convert, args) @mock.patch("skops.io._cli._convert") @@ -98,7 +98,7 @@ def test_with_trusted_works_as_expected( ): paths = ["abc.123", "234.567", "d/object_1.pkl"] args = paths + [trusted_flag] - skops.io._cli.main_convert(command_line_args=args) + skops.cli._convert.main_convert(command_line_args=args) self.assert_called_correctly(mock_convert, paths=paths, trusted=True) @mock.patch("skops.io._cli._convert") @@ -116,5 +116,5 @@ def test_with_output_dir_works_as_expected( else: args = paths - skops.io._cli.main_convert(command_line_args=args) + skops.cli._convert.main_convert(command_line_args=args) self.assert_called_correctly(mock_convert, paths=paths, output_dir=expected_dir) From aa240502bce76b76bf136a0f07bc2dc5dc3da791 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 17 Dec 2022 20:07:43 +0000 Subject: [PATCH 13/38] Address PR comments 1 --- skops/cli/__init__.py | 4 ++ skops/cli/_convert.py | 82 ++++++++++++++--------------- skops/cli/entrypoint.py | 2 +- skops/{io => cli}/tests/test_cli.py | 66 +++++++++++------------ skops/cli/tests/test_entrypoint.py | 7 ++- 5 files changed, 80 insertions(+), 81 deletions(-) rename skops/{io => cli}/tests/test_cli.py (55%) diff --git a/skops/cli/__init__.py b/skops/cli/__init__.py index e69de29b..93938a52 100644 --- a/skops/cli/__init__.py +++ b/skops/cli/__init__.py @@ -0,0 +1,4 @@ +from ._convert import _convert_file, main_convert +from .entrypoint import main_entrypoint + +__all__ = ["_convert_file", "main_convert", "main_entrypoint"] diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index 58565e2d..7890b98e 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -10,10 +10,22 @@ from skops.io import dumps, get_untrusted_types -def _convert( - input_file: os.PathLike, output_dir: pathlib.Path, is_trusted: bool = False -): - """Function that is called by the ``skops convert`` entrypoint""" +def _convert_file(input_file: os.PathLike, output_file: pathlib.Path): + """ + Function that is called by ``skops convert`` entrypoint. + + Loads a pickle model from the input path, converts to skops format, and saves to + output file. + + Parameters + ---------- + input_file : os.PathLike + Path of input .pkl model to load. + + output_file : os.PathLike + Path to save .skops model to. + + """ model_name = pathlib.Path(input_file).stem logging.info(f"Converting {model_name}") @@ -33,18 +45,12 @@ def _convert( "Unknown Types Detected!\n" f"While converting {model_name}, " "the following unsafe types were found: \n" - f"{untrusted_str}\n" + f"{untrusted_str}\n\n" + f"When loading{output_file}, add ``--trusted`` to the skops.load call." ) - if not is_trusted: - logging.warning( - f"Model {model_name} will not be converted due to unsafe types.\n" - "To convert this anyway, add `-t` to this command." - ) - return - - with open(output_dir / f"{model_name}.skops", "wb") as out_file: - logging.info(f"Writing to {output_dir / model_name}.skops") + with open(output_file, "wb") as out_file: + logging.info(f"Writing to {output_file}") out_file.write(skops_dump) @@ -60,15 +66,15 @@ def main_convert( parser.add_argument("inputs", nargs="+", help="Input files to convert.") parser.add_argument( - "-t", - "--trusted", + "-o", + "--output-files", + nargs="+", help=( - "Automatically trust all files, " - "and convert all inputs, " - "even if an untrusted type is detected." + "Specify output file names for the converted skops files." + "If not provided, will default to using the same name as the input file, " + "and saving to the current working directory." ), - action="store_true", - default=False, + default=None, ) parser.add_argument( "-d", @@ -87,28 +93,22 @@ def main_convert( dest="loglevel", const=logging.INFO, ) - parser.add_argument( - "--output-dir", - help=( - "Specify a directory to save converted files to. " - "Default will save files to the current working directory." - ), - type=str, - default=None, - ) - args = parser.parse_args(command_line_args) - - output_dir = args.output_dir - if not output_dir: - output_dir = pathlib.Path.cwd() - else: - output_dir = pathlib.Path(output_dir) + args = parser.parse_args(command_line_args) + output_files = args.output_files + input_files = args.inputs logging.basicConfig(format="%(message)s", level=args.loglevel) - for input_file in args.inputs: - _convert( + for input_file_index in range(len(args.inputs)): + input_file = input_files[input_file_index] + if output_files and len(output_files) >= input_file_index: + output_file = output_files[input_file_index] + else: + # No filename provided, defaulting to base file path + file_name = pathlib.Path(input_file).stem + output_file = pathlib.Path.cwd() / f"{file_name}.skops" + + _convert_file( input_file=input_file, - output_dir=output_dir, - is_trusted=args.trusted, + output_file=output_file, ) diff --git a/skops/cli/entrypoint.py b/skops/cli/entrypoint.py index 5e5c8335..9ef95c5c 100644 --- a/skops/cli/entrypoint.py +++ b/skops/cli/entrypoint.py @@ -11,7 +11,7 @@ def main_entrypoint(command_line_args=None): add_help=False, ) - function_map = {"convert": skops.cli._cli.main_convert} + function_map = {"convert": skops.cli._convert.main_convert} parser.add_argument( "function", help="Function to call.", choices=function_map.keys() ) diff --git a/skops/io/tests/test_cli.py b/skops/cli/tests/test_cli.py similarity index 55% rename from skops/io/tests/test_cli.py rename to skops/cli/tests/test_cli.py index 1a8ffaf2..ff2a31e7 100644 --- a/skops/io/tests/test_cli.py +++ b/skops/cli/tests/test_cli.py @@ -1,5 +1,4 @@ import logging -import os.path import pathlib import pickle as pkl from typing import Optional @@ -8,8 +7,8 @@ import numpy as np import pytest -import skops -from skops.cli._convert import _convert +from skops import cli +from skops.io import load class MockUnsafeType: @@ -49,16 +48,18 @@ def write_unsafe_file(self, pkl_path, unsafe_obj): def test_base_case_works_as_expected( self, pkl_path, tmp_path, skops_path, write_safe_file, safe_obj ): - _convert(pkl_path, tmp_path) - persisted_obj = skops.io.load(skops_path) + cli._convert_file(pkl_path, skops_path) + persisted_obj = load(skops_path) assert np.array_equal(persisted_obj, safe_obj) def test_unsafe_case_works_as_expected( self, pkl_path, tmp_path, skops_path, write_unsafe_file, caplog ): caplog.set_level(logging.WARNING) - _convert(pkl_path, tmp_path) - assert not os.path.isfile(skops_path) + cli._convert_file(pkl_path, skops_path) + persisted_obj = load(skops_path, trusted=True) + + assert isinstance(persisted_obj, MockUnsafeType) # check logging has warned that an unsafe type was found assert MockUnsafeType.__name__ in caplog.text @@ -69,52 +70,47 @@ class TestMainConvert: def assert_called_correctly( mock_convert: mock.MagicMock, paths: list, - output_dir: Optional[pathlib.Path] = pathlib.Path.cwd(), - trusted: Optional[bool] = False, + output_files: Optional[list] = None, ): + if not output_files: + output_files = [ + pathlib.Path.cwd() / f"{pathlib.Path(p).stem}.skops" for p in paths + ] assert mock_convert.call_count == len(paths) - mock_convert.assert_has_calls( [ - mock.call(input_file=p, output_dir=output_dir, is_trusted=trusted) - for p in paths + mock.call(input_file=paths[i], output_file=output_files[i]) + for i in range(len(paths)) ] ) - @mock.patch("skops.cli._convert._convert") + @mock.patch("skops.cli._convert._convert_file") def test_base_works_as_expected(self, mock_convert: mock.MagicMock): args = [ "123.pkl", "abc.pkl", ] - skops.cli._convert.main_convert(command_line_args=args) + cli.main_convert(command_line_args=args) self.assert_called_correctly(mock_convert, args) - @mock.patch("skops.io._cli._convert") - @pytest.mark.parametrize("trusted_flag", ["-t", "--trusted"]) - def test_with_trusted_works_as_expected( - self, mock_convert: mock.MagicMock, trusted_flag - ): - paths = ["abc.123", "234.567", "d/object_1.pkl"] - args = paths + [trusted_flag] - skops.cli._convert.main_convert(command_line_args=args) - self.assert_called_correctly(mock_convert, paths=paths, trusted=True) - - @mock.patch("skops.io._cli._convert") + @mock.patch("skops.cli._convert._convert_file") @pytest.mark.parametrize( - "output_dir, expected_dir", - [("a/b/c", pathlib.Path("a/b/c")), (None, pathlib.Path.cwd())], + "input_paths, output_files, expected_paths", + [ + (["abc.123"], ["a/b/c"], ["a/b/c"]), + (["abc.123"], None, [pathlib.Path.cwd() / "abc.skops"]), + ], ) def test_with_output_dir_works_as_expected( - self, mock_convert: mock.MagicMock, output_dir, expected_dir + self, mock_convert: mock.MagicMock, input_paths, output_files, expected_paths ): - paths = ["abc.123", "234.567", "d/object_1.pkl"] - - if output_dir is not None: - args = paths + ["--output-dir", output_dir] + if output_files is not None: + args = input_paths + ["--output-files"] + output_files else: - args = paths + args = input_paths - skops.cli._convert.main_convert(command_line_args=args) - self.assert_called_correctly(mock_convert, paths=paths, output_dir=expected_dir) + cli.main_convert(command_line_args=args) + self.assert_called_correctly( + mock_convert, paths=input_paths, output_files=expected_paths + ) diff --git a/skops/cli/tests/test_entrypoint.py b/skops/cli/tests/test_entrypoint.py index febf9164..51d065c7 100644 --- a/skops/cli/tests/test_entrypoint.py +++ b/skops/cli/tests/test_entrypoint.py @@ -19,14 +19,13 @@ def clear_argv(self): # Otherwise, clogs parser.parse_known_args() in argparse sys.argv = [""] - @mock.patch("skops.io._cli._convert") + @mock.patch("skops.cli._convert._convert_file") def test_convert_works_as_expected(self, mocked_convert: mock.MagicMock): - args = ["convert", "abc.def", "-t"] + args = ["convert", "abc.def"] main_entrypoint(args) mocked_convert.assert_called_once_with( input_file="abc.def", - output_dir=pathlib.Path.cwd(), - is_trusted=True, + output_file=pathlib.Path.cwd() / "abc.skops", ) From 02b3cf8d5f095a85f25b64867e67bd9a99dc7dee Mon Sep 17 00:00:00 2001 From: = Date: Sat, 17 Dec 2022 20:38:12 +0000 Subject: [PATCH 14/38] Add test to check for multiple output file types --- skops/cli/_convert.py | 2 +- skops/cli/tests/test_cli.py | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index 7890b98e..72590408 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -101,7 +101,7 @@ def main_convert( for input_file_index in range(len(args.inputs)): input_file = input_files[input_file_index] - if output_files and len(output_files) >= input_file_index: + if output_files and len(output_files) > input_file_index: output_file = output_files[input_file_index] else: # No filename provided, defaulting to base file path diff --git a/skops/cli/tests/test_cli.py b/skops/cli/tests/test_cli.py index ff2a31e7..6893b1bc 100644 --- a/skops/cli/tests/test_cli.py +++ b/skops/cli/tests/test_cli.py @@ -101,6 +101,7 @@ def test_base_works_as_expected(self, mock_convert: mock.MagicMock): (["abc.123"], ["a/b/c"], ["a/b/c"]), (["abc.123"], None, [pathlib.Path.cwd() / "abc.skops"]), ], + ids=["Given an output path", "No output path"], ) def test_with_output_dir_works_as_expected( self, mock_convert: mock.MagicMock, input_paths, output_files, expected_paths @@ -114,3 +115,28 @@ def test_with_output_dir_works_as_expected( self.assert_called_correctly( mock_convert, paths=input_paths, output_files=expected_paths ) + + @mock.patch("skops.cli._convert._convert_file") + @pytest.mark.parametrize( + "input_paths, output_files, expected_paths", + [ + ( + ["model_a.pkl", "model_b.pkl"], + ["a.skops", "b.skops"], + ["a.skops", "b.skops"], + ), + ( + ["model_a.pkl", "model_b.pkl", "model_c.pkl"], + ["a.skops", "b.skops"], + ["a.skops", "b.skops", pathlib.Path.cwd() / "model_c.skops"], + ), + ], + ids=["With enough output paths", "With only some output paths"], + ) + def test_for_multiple_inputs_and_outputs_works_as_expected( + self, mock_convert: mock.MagicMock, input_paths, output_files, expected_paths + ): + args = input_paths + ["--output-files"] + output_files + cli.main_convert(args) + + self.assert_called_correctly(mock_convert, input_paths, expected_paths) From 245197f019d9d5cf2cc69e7f2d6127b3a3eae2d3 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 17 Dec 2022 20:49:30 +0000 Subject: [PATCH 15/38] Update entrypoint name in setup.py --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 535162fe..7df920c6 100644 --- a/setup.py +++ b/setup.py @@ -37,7 +37,7 @@ def setup_package(): package_data = dict( entry_points={ "console_scripts": [ - "skops = skops.utils.entrypoint:main_entrypoint", + "skops = skops.cli.entrypoint:main_entrypoint", ], } ) From 2b681ced8fe7ca7de8d194057a4ee4fd05a0ab2f Mon Sep 17 00:00:00 2001 From: = Date: Tue, 3 Jan 2023 21:23:58 +0000 Subject: [PATCH 16/38] PR comments before reworking to support only single files --- skops/cli/__init__.py | 4 ---- skops/cli/_convert.py | 12 ++++++------ skops/cli/tests/test_cli.py | 12 ++++++------ 3 files changed, 12 insertions(+), 16 deletions(-) delete mode 100644 skops/cli/__init__.py diff --git a/skops/cli/__init__.py b/skops/cli/__init__.py deleted file mode 100644 index 93938a52..00000000 --- a/skops/cli/__init__.py +++ /dev/null @@ -1,4 +0,0 @@ -from ._convert import _convert_file, main_convert -from .entrypoint import main_entrypoint - -__all__ = ["_convert_file", "main_convert", "main_entrypoint"] diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index 72590408..c54afd4c 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -4,13 +4,13 @@ import logging import os import pathlib -import pickle as pkl +import pickle from typing import Optional from skops.io import dumps, get_untrusted_types -def _convert_file(input_file: os.PathLike, output_file: pathlib.Path): +def _convert_file(input_file: os.PathLike, output_file: os.PathLike): """ Function that is called by ``skops convert`` entrypoint. @@ -31,22 +31,22 @@ def _convert_file(input_file: os.PathLike, output_file: pathlib.Path): logging.info(f"Converting {model_name}") with open(input_file, "rb") as f: - obj = pkl.load(f) + obj = pickle.load(f) skops_dump = dumps(obj) untrusted_types = get_untrusted_types(data=skops_dump) if not untrusted_types: - logging.debug(f"No unsafe types found in {model_name}.") + logging.debug(f"No unknown types found in {model_name}.") else: untrusted_str = "\n".join(untrusted_types) logging.warning( "Unknown Types Detected!\n" f"While converting {model_name}, " - "the following unsafe types were found: \n" + "the following unknown types were found: \n" f"{untrusted_str}\n\n" - f"When loading{output_file}, add ``--trusted`` to the skops.load call." + f"When loading {output_file}, add ``trusted=True`` to the skops.load call." ) with open(output_file, "wb") as out_file: diff --git a/skops/cli/tests/test_cli.py b/skops/cli/tests/test_cli.py index 6893b1bc..80234fb2 100644 --- a/skops/cli/tests/test_cli.py +++ b/skops/cli/tests/test_cli.py @@ -7,7 +7,7 @@ import numpy as np import pytest -from skops import cli +from skops.cli import _convert from skops.io import load @@ -48,7 +48,7 @@ def write_unsafe_file(self, pkl_path, unsafe_obj): def test_base_case_works_as_expected( self, pkl_path, tmp_path, skops_path, write_safe_file, safe_obj ): - cli._convert_file(pkl_path, skops_path) + _convert._convert_file(pkl_path, skops_path) persisted_obj = load(skops_path) assert np.array_equal(persisted_obj, safe_obj) @@ -56,7 +56,7 @@ def test_unsafe_case_works_as_expected( self, pkl_path, tmp_path, skops_path, write_unsafe_file, caplog ): caplog.set_level(logging.WARNING) - cli._convert_file(pkl_path, skops_path) + _convert._convert_file(pkl_path, skops_path) persisted_obj = load(skops_path, trusted=True) assert isinstance(persisted_obj, MockUnsafeType) @@ -91,7 +91,7 @@ def test_base_works_as_expected(self, mock_convert: mock.MagicMock): "abc.pkl", ] - cli.main_convert(command_line_args=args) + _convert.main_convert(command_line_args=args) self.assert_called_correctly(mock_convert, args) @mock.patch("skops.cli._convert._convert_file") @@ -111,7 +111,7 @@ def test_with_output_dir_works_as_expected( else: args = input_paths - cli.main_convert(command_line_args=args) + _convert.main_convert(command_line_args=args) self.assert_called_correctly( mock_convert, paths=input_paths, output_files=expected_paths ) @@ -137,6 +137,6 @@ def test_for_multiple_inputs_and_outputs_works_as_expected( self, mock_convert: mock.MagicMock, input_paths, output_files, expected_paths ): args = input_paths + ["--output-files"] + output_files - cli.main_convert(args) + _convert.main_convert(args) self.assert_called_correctly(mock_convert, input_paths, expected_paths) From 25b539c4ec66175bb6ed2c838c0dbaa9019c79ef Mon Sep 17 00:00:00 2001 From: = Date: Wed, 4 Jan 2023 20:25:00 +0000 Subject: [PATCH 17/38] Pre-parser rework] --- setup.py | 2 +- skops/cli/_convert.py | 22 ++++++++++++--------- skops/cli/entrypoint.py | 18 ++++++++++++----- skops/cli/tests/__init__.py | 0 skops/cli/tests/test_cli.py | 9 +++++---- skops/cli/tests/test_entrypoint.py | 31 ------------------------------ 6 files changed, 32 insertions(+), 50 deletions(-) delete mode 100644 skops/cli/tests/__init__.py diff --git a/setup.py b/setup.py index 7df920c6..ce368f12 100644 --- a/setup.py +++ b/setup.py @@ -37,7 +37,7 @@ def setup_package(): package_data = dict( entry_points={ "console_scripts": [ - "skops = skops.cli.entrypoint:main_entrypoint", + "skops = skops.cli.entrypoint:main_cli", ], } ) diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index c54afd4c..aa434c3c 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -54,17 +54,12 @@ def _convert_file(input_file: os.PathLike, output_file: os.PathLike): out_file.write(skops_dump) -def main_convert( - command_line_args: Optional[list[str]] = None, - parent: Optional[argparse.ArgumentParser] = None, -): - parents = [parent] if parent else [] - +def get_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser( - description="Convert input Pickle files to .skops files", parents=parents + description="Convert input Pickle files to .skops files" ) - parser.add_argument("inputs", nargs="+", help="Input files to convert.") + parser.add_argument("inputs", nargs="+", help="Input files to convert. ") parser.add_argument( "-o", "--output-files", @@ -72,7 +67,7 @@ def main_convert( help=( "Specify output file names for the converted skops files." "If not provided, will default to using the same name as the input file, " - "and saving to the current working directory." + "and saving to the current working directory with the suffix ``.skops``. " ), default=None, ) @@ -93,6 +88,15 @@ def main_convert( dest="loglevel", const=logging.INFO, ) + return parser + + +def main( + command_line_args: Optional[list[str]] = None, + parser: Optional[argparse.ArgumentParser] = None, +): + if not parser: + parser = get_parser() args = parser.parse_args(command_line_args) output_files = args.output_files diff --git a/skops/cli/entrypoint.py b/skops/cli/entrypoint.py index 9ef95c5c..032610c8 100644 --- a/skops/cli/entrypoint.py +++ b/skops/cli/entrypoint.py @@ -3,17 +3,25 @@ import skops.cli._convert -def main_entrypoint(command_line_args=None): - """Main entrypoint for all command line Skops methods.""" +def main_cli(command_line_args=None): + """Main command line interface entrypoint for all command line Skops methods.""" parser = argparse.ArgumentParser( prog="Skops", description="Main entrypoint for all command line Skops methods.", - add_help=False, + add_help=True, ) - function_map = {"convert": skops.cli._convert.main_convert} + # NB: methods should be functions, parsers should be objects + function_map = { + "convert": { + "method": skops.cli._convert.main, + "parser": skops.cli._convert.get_parser, + }, + } + parser.add_argument( "function", help="Function to call.", choices=function_map.keys() ) + # subparsers = parser.add_subparsers(help="sub-command help") + args, _ = parser.parse_known_args(command_line_args) - function_map.get(args.function)(parent=parser, command_line_args=command_line_args) diff --git a/skops/cli/tests/__init__.py b/skops/cli/tests/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/skops/cli/tests/test_cli.py b/skops/cli/tests/test_cli.py index 80234fb2..f15d94bd 100644 --- a/skops/cli/tests/test_cli.py +++ b/skops/cli/tests/test_cli.py @@ -46,11 +46,12 @@ def write_unsafe_file(self, pkl_path, unsafe_obj): pkl.dump(unsafe_obj, f) def test_base_case_works_as_expected( - self, pkl_path, tmp_path, skops_path, write_safe_file, safe_obj + self, pkl_path, tmp_path, skops_path, write_safe_file, safe_obj, caplog ): _convert._convert_file(pkl_path, skops_path) persisted_obj = load(skops_path) assert np.array_equal(persisted_obj, safe_obj) + assert MockUnsafeType.__name__ not in caplog.text def test_unsafe_case_works_as_expected( self, pkl_path, tmp_path, skops_path, write_unsafe_file, caplog @@ -91,7 +92,7 @@ def test_base_works_as_expected(self, mock_convert: mock.MagicMock): "abc.pkl", ] - _convert.main_convert(command_line_args=args) + _convert.main(command_line_args=args) self.assert_called_correctly(mock_convert, args) @mock.patch("skops.cli._convert._convert_file") @@ -111,7 +112,7 @@ def test_with_output_dir_works_as_expected( else: args = input_paths - _convert.main_convert(command_line_args=args) + _convert.main(command_line_args=args) self.assert_called_correctly( mock_convert, paths=input_paths, output_files=expected_paths ) @@ -137,6 +138,6 @@ def test_for_multiple_inputs_and_outputs_works_as_expected( self, mock_convert: mock.MagicMock, input_paths, output_files, expected_paths ): args = input_paths + ["--output-files"] + output_files - _convert.main_convert(args) + _convert.main(args) self.assert_called_correctly(mock_convert, input_paths, expected_paths) diff --git a/skops/cli/tests/test_entrypoint.py b/skops/cli/tests/test_entrypoint.py index 51d065c7..e69de29b 100644 --- a/skops/cli/tests/test_entrypoint.py +++ b/skops/cli/tests/test_entrypoint.py @@ -1,31 +0,0 @@ -import pathlib -import sys -from unittest import mock - -import pytest - -from skops.cli.entrypoint import main_entrypoint - - -class TestEntrypoint: - """Integration tests that check that entrypoint calls pass through correctly. - - Full coverage of individual entrypoint calls should be done in their own classes. - """ - - @pytest.fixture(autouse=True) - def clear_argv(self): - # Required to clear argv in case Pytest is called on this specific function. - # Otherwise, clogs parser.parse_known_args() in argparse - sys.argv = [""] - - @mock.patch("skops.cli._convert._convert_file") - def test_convert_works_as_expected(self, mocked_convert: mock.MagicMock): - args = ["convert", "abc.def"] - - main_entrypoint(args) - - mocked_convert.assert_called_once_with( - input_file="abc.def", - output_file=pathlib.Path.cwd() / "abc.skops", - ) From 50bf18916548b89e0f316c5d90d1f87e0778a3af Mon Sep 17 00:00:00 2001 From: = Date: Wed, 4 Jan 2023 20:25:34 +0000 Subject: [PATCH 18/38] Remove empty test file --- skops/cli/tests/test_entrypoint.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 skops/cli/tests/test_entrypoint.py diff --git a/skops/cli/tests/test_entrypoint.py b/skops/cli/tests/test_entrypoint.py deleted file mode 100644 index e69de29b..00000000 From 227f23cbe18cc9dc576ed56f6cbb08f78aae17d9 Mon Sep 17 00:00:00 2001 From: = Date: Mon, 9 Jan 2023 20:51:20 +0000 Subject: [PATCH 19/38] Pre-test reintroduction --- skops/cli/_convert.py | 88 ++++++++++++++++++------------------- skops/cli/entrypoint.py | 29 ++++++++---- skops/cli/tests/test_cli.py | 69 +++++++---------------------- 3 files changed, 80 insertions(+), 106 deletions(-) diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index aa434c3c..a4875bc9 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -28,7 +28,7 @@ def _convert_file(input_file: os.PathLike, output_file: os.PathLike): """ model_name = pathlib.Path(input_file).stem - logging.info(f"Converting {model_name}") + logging.debug(f"Converting {model_name}") with open(input_file, "rb") as f: obj = pickle.load(f) @@ -37,41 +37,53 @@ def _convert_file(input_file: os.PathLike, output_file: os.PathLike): untrusted_types = get_untrusted_types(data=skops_dump) if not untrusted_types: - logging.debug(f"No unknown types found in {model_name}.") + logging.info(f"No unknown types found in {model_name}.") else: - untrusted_str = "\n".join(untrusted_types) + untrusted_str = ", ".join(untrusted_types) logging.warning( - "Unknown Types Detected!\n" + "Unknown Types Detected! " f"While converting {model_name}, " - "the following unknown types were found: \n" - f"{untrusted_str}\n\n" - f"When loading {output_file}, add ``trusted=True`` to the skops.load call." + "the following unknown types were found: " + f"{untrusted_str}. " + f"When loading {output_file}, add 'trusted=True' to the skops.load call. " ) with open(output_file, "wb") as out_file: - logging.info(f"Writing to {output_file}") + logging.debug(f"Writing to {output_file}") out_file.write(skops_dump) -def get_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser( - description="Convert input Pickle files to .skops files" - ) +def format_parser( + parser: Optional[argparse.ArgumentParser] = None, +) -> argparse.ArgumentParser: + """Adds arguments and help to parent CLI parser for the convert method.""" + + if not parser: # used in tests + parser = argparse.ArgumentParser() - parser.add_argument("inputs", nargs="+", help="Input files to convert. ") - parser.add_argument( + parser_subgroup = parser.add_argument_group("convert") + parser_subgroup.add_argument("input", help="Input file to convert. ") + + parser_subgroup.add_argument( "-o", - "--output-files", - nargs="+", + "--output-file", help=( - "Specify output file names for the converted skops files." + "Specify the output file name for the converted skops file. " "If not provided, will default to using the same name as the input file, " - "and saving to the current working directory with the suffix ``.skops``. " + "and saving to the current working directory with the suffix '.skops'." ), default=None, ) - parser.add_argument( + parser_subgroup.add_argument( + "-v", + "--verbose", + help="Enable verbose logging.", + action="store_const", + dest="loglevel", + const=logging.INFO, + ) + parser_subgroup.add_argument( "-d", "--debug", help="Enable debug logging.", @@ -80,14 +92,6 @@ def get_parser() -> argparse.ArgumentParser: const=logging.DEBUG, default=logging.WARNING, ) - parser.add_argument( - "-v", - "--verbose", - help="Enable verbose logging.", - action="store_const", - dest="loglevel", - const=logging.INFO, - ) return parser @@ -96,23 +100,19 @@ def main( parser: Optional[argparse.ArgumentParser] = None, ): if not parser: - parser = get_parser() + parser = format_parser(argparse.ArgumentParser()) - args = parser.parse_args(command_line_args) - output_files = args.output_files - input_files = args.inputs + args, _ = parser.parse_known_args(command_line_args) + output_file = args.output_file + input_file = args.input logging.basicConfig(format="%(message)s", level=args.loglevel) - for input_file_index in range(len(args.inputs)): - input_file = input_files[input_file_index] - if output_files and len(output_files) > input_file_index: - output_file = output_files[input_file_index] - else: - # No filename provided, defaulting to base file path - file_name = pathlib.Path(input_file).stem - output_file = pathlib.Path.cwd() / f"{file_name}.skops" - - _convert_file( - input_file=input_file, - output_file=output_file, - ) + if not output_file: + # No filename provided, defaulting to base file path + file_name = pathlib.Path(input_file).stem + output_file = pathlib.Path.cwd() / f"{file_name}.skops" + + _convert_file( + input_file=input_file, + output_file=output_file, + ) diff --git a/skops/cli/entrypoint.py b/skops/cli/entrypoint.py index 032610c8..cdcb5608 100644 --- a/skops/cli/entrypoint.py +++ b/skops/cli/entrypoint.py @@ -4,24 +4,35 @@ def main_cli(command_line_args=None): - """Main command line interface entrypoint for all command line Skops methods.""" - parser = argparse.ArgumentParser( + """ + Main command line interface entrypoint for all command line Skops methods. + + To add a new entrypoint: + 1. Create a new main + """ + entry_parser = argparse.ArgumentParser( prog="Skops", description="Main entrypoint for all command line Skops methods.", add_help=True, ) - # NB: methods should be functions, parsers should be objects + subparsers = entry_parser.add_subparsers( + title="Command", + description="Skops command to call", + dest="cmd", + help="Sub-commands help", + ) + function_map = { "convert": { "method": skops.cli._convert.main, - "parser": skops.cli._convert.get_parser, + "format_parser": skops.cli._convert.format_parser, }, } - parser.add_argument( - "function", help="Function to call.", choices=function_map.keys() - ) - # subparsers = parser.add_subparsers(help="sub-command help") + for func_name, values in function_map.items(): + subparser = subparsers.add_parser(func_name) + subparser.set_defaults(func=values["method"]) + values["format_parser"](subparser) - args, _ = parser.parse_known_args(command_line_args) + args, _ = entry_parser.parse_known_args(command_line_args) diff --git a/skops/cli/tests/test_cli.py b/skops/cli/tests/test_cli.py index f15d94bd..b4838665 100644 --- a/skops/cli/tests/test_cli.py +++ b/skops/cli/tests/test_cli.py @@ -1,7 +1,6 @@ import logging import pathlib import pickle as pkl -from typing import Optional from unittest import mock import numpy as np @@ -70,74 +69,38 @@ class TestMainConvert: @staticmethod def assert_called_correctly( mock_convert: mock.MagicMock, - paths: list, - output_files: Optional[list] = None, + path, + output_file=None, ): - if not output_files: - output_files = [ - pathlib.Path.cwd() / f"{pathlib.Path(p).stem}.skops" for p in paths - ] - assert mock_convert.call_count == len(paths) - mock_convert.assert_has_calls( - [ - mock.call(input_file=paths[i], output_file=output_files[i]) - for i in range(len(paths)) - ] - ) + if not output_file: + output_file = pathlib.Path.cwd() / f"{pathlib.Path(path).stem}.skops" + mock_convert.assert_called_once_with(input_file=path, output_file=output_file) @mock.patch("skops.cli._convert._convert_file") def test_base_works_as_expected(self, mock_convert: mock.MagicMock): - args = [ - "123.pkl", - "abc.pkl", - ] + path = "123.pkl" - _convert.main(command_line_args=args) - self.assert_called_correctly(mock_convert, args) + _convert.main(command_line_args=[path]) + self.assert_called_correctly(mock_convert, path) @mock.patch("skops.cli._convert._convert_file") @pytest.mark.parametrize( - "input_paths, output_files, expected_paths", + "input_path, output_file, expected_path", [ - (["abc.123"], ["a/b/c"], ["a/b/c"]), - (["abc.123"], None, [pathlib.Path.cwd() / "abc.skops"]), + ("abc.123", "a/b/c", "a/b/c"), + ("abc.123", None, pathlib.Path.cwd() / "abc.skops"), ], ids=["Given an output path", "No output path"], ) def test_with_output_dir_works_as_expected( - self, mock_convert: mock.MagicMock, input_paths, output_files, expected_paths + self, mock_convert: mock.MagicMock, input_path, output_file, expected_path ): - if output_files is not None: - args = input_paths + ["--output-files"] + output_files + if output_file is not None: + args = [input_path, "--output", output_file] else: - args = input_paths + args = [input_path] _convert.main(command_line_args=args) self.assert_called_correctly( - mock_convert, paths=input_paths, output_files=expected_paths + mock_convert, path=input_path, output_file=expected_path ) - - @mock.patch("skops.cli._convert._convert_file") - @pytest.mark.parametrize( - "input_paths, output_files, expected_paths", - [ - ( - ["model_a.pkl", "model_b.pkl"], - ["a.skops", "b.skops"], - ["a.skops", "b.skops"], - ), - ( - ["model_a.pkl", "model_b.pkl", "model_c.pkl"], - ["a.skops", "b.skops"], - ["a.skops", "b.skops", pathlib.Path.cwd() / "model_c.skops"], - ), - ], - ids=["With enough output paths", "With only some output paths"], - ) - def test_for_multiple_inputs_and_outputs_works_as_expected( - self, mock_convert: mock.MagicMock, input_paths, output_files, expected_paths - ): - args = input_paths + ["--output-files"] + output_files - _convert.main(args) - - self.assert_called_correctly(mock_convert, input_paths, expected_paths) From 426d663ca5e387230a129faecf6866c29207b3ff Mon Sep 17 00:00:00 2001 From: = Date: Mon, 9 Jan 2023 22:06:06 +0000 Subject: [PATCH 20/38] Pre-test reintroduction --- skops/cli/_convert.py | 13 ++++------- skops/cli/entrypoint.py | 5 ++-- skops/cli/tests/test_cli.py | 7 ++++-- skops/cli/tests/test_entrypoint.py | 37 ++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 13 deletions(-) create mode 100644 skops/cli/tests/test_entrypoint.py diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index a4875bc9..6852d81b 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -96,16 +96,11 @@ def format_parser( def main( - command_line_args: Optional[list[str]] = None, - parser: Optional[argparse.ArgumentParser] = None, + parsed_args: argparse.Namespace, ): - if not parser: - parser = format_parser(argparse.ArgumentParser()) - - args, _ = parser.parse_known_args(command_line_args) - output_file = args.output_file - input_file = args.input - logging.basicConfig(format="%(message)s", level=args.loglevel) + output_file = parsed_args.output_file + input_file = parsed_args.input + logging.basicConfig(format="%(message)s", level=parsed_args.loglevel) if not output_file: # No filename provided, defaulting to base file path diff --git a/skops/cli/entrypoint.py b/skops/cli/entrypoint.py index cdcb5608..ed50c445 100644 --- a/skops/cli/entrypoint.py +++ b/skops/cli/entrypoint.py @@ -17,7 +17,7 @@ def main_cli(command_line_args=None): ) subparsers = entry_parser.add_subparsers( - title="Command", + title="Commands", description="Skops command to call", dest="cmd", help="Sub-commands help", @@ -35,4 +35,5 @@ def main_cli(command_line_args=None): subparser.set_defaults(func=values["method"]) values["format_parser"](subparser) - args, _ = entry_parser.parse_known_args(command_line_args) + args = entry_parser.parse_args(command_line_args) + args.func(args) diff --git a/skops/cli/tests/test_cli.py b/skops/cli/tests/test_cli.py index b4838665..fe76953e 100644 --- a/skops/cli/tests/test_cli.py +++ b/skops/cli/tests/test_cli.py @@ -79,8 +79,9 @@ def assert_called_correctly( @mock.patch("skops.cli._convert._convert_file") def test_base_works_as_expected(self, mock_convert: mock.MagicMock): path = "123.pkl" + namespace, _ = _convert.format_parser().parse_known_args([path]) - _convert.main(command_line_args=[path]) + _convert.main(namespace) self.assert_called_correctly(mock_convert, path) @mock.patch("skops.cli._convert._convert_file") @@ -100,7 +101,9 @@ def test_with_output_dir_works_as_expected( else: args = [input_path] - _convert.main(command_line_args=args) + namespace, _ = _convert.format_parser().parse_known_args(args) + + _convert.main(namespace) self.assert_called_correctly( mock_convert, path=input_path, output_file=expected_path ) diff --git a/skops/cli/tests/test_entrypoint.py b/skops/cli/tests/test_entrypoint.py new file mode 100644 index 00000000..0947307a --- /dev/null +++ b/skops/cli/tests/test_entrypoint.py @@ -0,0 +1,37 @@ +import pathlib +import sys +from unittest import mock + +import pytest + +from skops.cli.entrypoint import main_cli + + +class TestEntrypoint: + """Integration tests that check that entrypoint calls pass through correctly. + Full coverage of individual entrypoint calls should be done in their own classes. + """ + + @pytest.fixture(autouse=True) + def clear_argv(self): + # Required to clear argv in case Pytest is called on this specific function. + # Otherwise, clogs parser.parse_known_args() in argparse + sys.argv = [""] + + @mock.patch("skops.cli._convert._convert_file") + def test_convert_works_as_expected( + self, + convert_file_mock: mock.MagicMock, + ): + """ + Intended as a unit test to make sure, + given 'convert' as the first argument, + the parser is configured correctly + """ + + args = ["convert", "abc.def"] + + main_cli(args) + convert_file_mock.assert_called_once_with( + input_file="abc.def", output_file=pathlib.Path.cwd() / "abc.skops" + ) From 37d3a871ac8f5459c887e0320c17baf55b1651f5 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 10 Jan 2023 09:01:32 +0000 Subject: [PATCH 21/38] Change verbosity to work as discussed in PR --- skops/cli/_convert.py | 17 +++------- skops/cli/_utils.py | 15 +++++++++ .../tests/{test_cli.py => test_convert.py} | 31 ++++++++++++++++++- skops/cli/tests/test_entrypoint.py | 4 +++ 4 files changed, 54 insertions(+), 13 deletions(-) create mode 100644 skops/cli/_utils.py rename skops/cli/tests/{test_cli.py => test_convert.py} (76%) diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index 6852d81b..9000f57b 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -7,6 +7,7 @@ import pickle from typing import Optional +from skops.cli._utils import get_log_level from skops.io import dumps, get_untrusted_types @@ -79,18 +80,9 @@ def format_parser( "-v", "--verbose", help="Enable verbose logging.", - action="store_const", + action="count", dest="loglevel", - const=logging.INFO, - ) - parser_subgroup.add_argument( - "-d", - "--debug", - help="Enable debug logging.", - action="store_const", - dest="loglevel", - const=logging.DEBUG, - default=logging.WARNING, + default=0, ) return parser @@ -100,7 +92,8 @@ def main( ): output_file = parsed_args.output_file input_file = parsed_args.input - logging.basicConfig(format="%(message)s", level=parsed_args.loglevel) + + logging.basicConfig(format="%(message)s", level=get_log_level(parsed_args.loglevel)) if not output_file: # No filename provided, defaulting to base file path diff --git a/skops/cli/_utils.py b/skops/cli/_utils.py new file mode 100644 index 00000000..82a371ab --- /dev/null +++ b/skops/cli/_utils.py @@ -0,0 +1,15 @@ +import logging + + +def get_log_level(level: int = 0): + """Takes in verbosity from a CLI entrypoint (number of times -v specified), + and sets the logger to the required log level""" + + all_levels = [logging.WARNING, logging.INFO, logging.DEBUG] + + if level > len(all_levels): + level = len(all_levels) + elif level < 0: + level = 0 + + return all_levels[level] diff --git a/skops/cli/tests/test_cli.py b/skops/cli/tests/test_convert.py similarity index 76% rename from skops/cli/tests/test_cli.py rename to skops/cli/tests/test_convert.py index fe76953e..049d44b8 100644 --- a/skops/cli/tests/test_cli.py +++ b/skops/cli/tests/test_convert.py @@ -65,7 +65,7 @@ def test_unsafe_case_works_as_expected( assert MockUnsafeType.__name__ in caplog.text -class TestMainConvert: +class TestMain: @staticmethod def assert_called_correctly( mock_convert: mock.MagicMock, @@ -107,3 +107,32 @@ def test_with_output_dir_works_as_expected( self.assert_called_correctly( mock_convert, path=input_path, output_file=expected_path ) + + @mock.patch("skops.cli._convert._convert_file") + @pytest.mark.parametrize( + "verbosity, expected_level", + [ + ("", logging.WARNING), + ("-v", logging.INFO), + ("--verbose", logging.INFO), + ("-vv", logging.DEBUG), + ("-v -v", logging.DEBUG), + ("-vvv", logging.DEBUG), + ("--verbose --verbose", logging.DEBUG), + ], + ) + def test_given_log_levels_works_as_expected( + self, mock_convert: mock.MagicMock, verbosity, expected_level, caplog + ): + input_path = "abc.def" + output_path = "bde.skops" + args = [input_path, "--output", output_path, verbosity.split()] + + namespace, _ = _convert.format_parser().parse_known_args(args) + + _convert.main(namespace) + self.assert_called_correctly( + mock_convert, path=input_path, output_file=output_path + ) + + assert caplog.at_level(expected_level) diff --git a/skops/cli/tests/test_entrypoint.py b/skops/cli/tests/test_entrypoint.py index 0947307a..2fd3cb60 100644 --- a/skops/cli/tests/test_entrypoint.py +++ b/skops/cli/tests/test_entrypoint.py @@ -1,3 +1,4 @@ +import logging import pathlib import sys from unittest import mock @@ -22,6 +23,7 @@ def clear_argv(self): def test_convert_works_as_expected( self, convert_file_mock: mock.MagicMock, + caplog, ): """ Intended as a unit test to make sure, @@ -35,3 +37,5 @@ def test_convert_works_as_expected( convert_file_mock.assert_called_once_with( input_file="abc.def", output_file=pathlib.Path.cwd() / "abc.skops" ) + + assert caplog.at_level(logging.WARNING) From 9b9f342bc775fdc3ed2cfbf4ffd6bfc23deccea0 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 10 Jan 2023 09:37:30 +0000 Subject: [PATCH 22/38] Add needed __init__ file --- skops/cli/__init__.py | 0 skops/cli/_convert.py | 5 ++++- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 skops/cli/__init__.py diff --git a/skops/cli/__init__.py b/skops/cli/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index 9000f57b..ca2f0548 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -79,7 +79,10 @@ def format_parser( parser_subgroup.add_argument( "-v", "--verbose", - help="Enable verbose logging.", + help=( + "Increases verbosity of logging. Can be used multiple times to increase " + "further." + ), action="count", dest="loglevel", default=0, From 6f5b71e377a81e9b7237006d18b241a76afe2509 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 10 Jan 2023 09:40:41 +0000 Subject: [PATCH 23/38] Update docstrings --- skops/cli/entrypoint.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/skops/cli/entrypoint.py b/skops/cli/entrypoint.py index ed50c445..ec7c0305 100644 --- a/skops/cli/entrypoint.py +++ b/skops/cli/entrypoint.py @@ -8,7 +8,9 @@ def main_cli(command_line_args=None): Main command line interface entrypoint for all command line Skops methods. To add a new entrypoint: - 1. Create a new main + 1. Create a new method to call that accepts a namespace + 2. Create a new subparser formatter to define the expected CL arguments + 3. Add those to the function map. """ entry_parser = argparse.ArgumentParser( prog="Skops", @@ -23,6 +25,9 @@ def main_cli(command_line_args=None): help="Sub-commands help", ) + # function_map should map the expected CL command to + # method: the command to call + # format_parser: the function used to create a subparser for that command function_map = { "convert": { "method": skops.cli._convert.main, From aa7c3ba5205a88fce78b3c5036c19a214d898971 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 10 Jan 2023 09:44:53 +0000 Subject: [PATCH 24/38] Make names of files uniform --- skops/cli/_convert.py | 4 ++-- skops/cli/entrypoint.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index ca2f0548..594edaf9 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -64,7 +64,7 @@ def format_parser( parser = argparse.ArgumentParser() parser_subgroup = parser.add_argument_group("convert") - parser_subgroup.add_argument("input", help="Input file to convert. ") + parser_subgroup.add_argument("input-file", help="Input file to convert. ") parser_subgroup.add_argument( "-o", @@ -94,7 +94,7 @@ def main( parsed_args: argparse.Namespace, ): output_file = parsed_args.output_file - input_file = parsed_args.input + input_file = parsed_args.input_file logging.basicConfig(format="%(message)s", level=get_log_level(parsed_args.loglevel)) diff --git a/skops/cli/entrypoint.py b/skops/cli/entrypoint.py index ec7c0305..f2a7ac17 100644 --- a/skops/cli/entrypoint.py +++ b/skops/cli/entrypoint.py @@ -25,7 +25,7 @@ def main_cli(command_line_args=None): help="Sub-commands help", ) - # function_map should map the expected CL command to + # function_map should map a command to # method: the command to call # format_parser: the function used to create a subparser for that command function_map = { From 1c0352d359723b56ca6228d120ffa55ecd9f7356 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 10 Jan 2023 18:55:01 +0000 Subject: [PATCH 25/38] Update help docstrings --- docs/changes.rst | 9 ++++++++- skops/cli/_convert.py | 6 +++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index f1aa7739..86f78ded 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -9,6 +9,12 @@ skops Changelog :depth: 1 :local: +v0.5 +---- +- Added CLI entrypoint support (:func:`.cli.entrypoint.main_cli`) + and a command line function to convert Pickle files + to Skops files (:func:`.cli._convert.main`). :pr:`249` by `Erin Aho`_ + v0.4 ---- - :func:`.io.dump` and :func:`.io.load` now work with file like objects, @@ -83,4 +89,5 @@ Contributors :user:`Adrin Jalali `, :user:`Merve Noyan `, :user:`Benjamin Bossan `, :user:`Ayyuce Demirbas -`, :user:`Prajjwal Mishra ` +`, :user:`Prajjwal Mishra `, +:user:`Erin Aho `, diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index 594edaf9..e68d2760 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -64,7 +64,7 @@ def format_parser( parser = argparse.ArgumentParser() parser_subgroup = parser.add_argument_group("convert") - parser_subgroup.add_argument("input-file", help="Input file to convert. ") + parser_subgroup.add_argument("input", help="Path to an input file to convert. ") parser_subgroup.add_argument( "-o", @@ -81,7 +81,7 @@ def format_parser( "--verbose", help=( "Increases verbosity of logging. Can be used multiple times to increase " - "further." + "verbosity further." ), action="count", dest="loglevel", @@ -94,7 +94,7 @@ def main( parsed_args: argparse.Namespace, ): output_file = parsed_args.output_file - input_file = parsed_args.input_file + input_file = parsed_args.input logging.basicConfig(format="%(message)s", level=get_log_level(parsed_args.loglevel)) From 8d9b3c52de0a3d5069dc7b94e31e36cd9dc0f717 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 10 Jan 2023 19:07:30 +0000 Subject: [PATCH 26/38] Add documentation for CLI --- docs/persistence.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/persistence.rst b/docs/persistence.rst index 07aa3e2c..0c2871ad 100644 --- a/docs/persistence.rst +++ b/docs/persistence.rst @@ -110,6 +110,22 @@ you have custom functions (say, a custom function to be used with most ``numpy`` and ``scipy`` functions should work. Therefore, you can save objects having references to functions such as ``numpy.sqrt``. +Command Line Interface +######## +Skops has a command line interface to convert SciKit-Learn models persisted with +``Pickle`` to ``Skops`` files. + +To convert a file from the command line, use the ``skops convert`` entrypoint. + +Below is an example call to convert a file ``my_model.pkl`` to ``new_model.skops``: + +.. code:: console + skops convert my_model.pkl + +Further help for the different supported options can be found by calling +``skops convert --help`` in a terminal. + + Supported libraries ------------------- From 8150f9a429941a5dae084c1f77ae092435639e59 Mon Sep 17 00:00:00 2001 From: Erin Aho Date: Tue, 17 Jan 2023 11:17:22 +0000 Subject: [PATCH 27/38] Update docs/persistence.rst Co-authored-by: Benjamin Bossan --- docs/persistence.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/persistence.rst b/docs/persistence.rst index 0c2871ad..86fed026 100644 --- a/docs/persistence.rst +++ b/docs/persistence.rst @@ -111,7 +111,8 @@ most ``numpy`` and ``scipy`` functions should work. Therefore, you can save objects having references to functions such as ``numpy.sqrt``. Command Line Interface -######## +###################### + Skops has a command line interface to convert SciKit-Learn models persisted with ``Pickle`` to ``Skops`` files. From 88e7be05e0eaa5e73ab9bc490d592c9d4217cd4b Mon Sep 17 00:00:00 2001 From: Erin Aho Date: Tue, 17 Jan 2023 11:17:35 +0000 Subject: [PATCH 28/38] Update docs/persistence.rst Co-authored-by: Benjamin Bossan --- docs/persistence.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/persistence.rst b/docs/persistence.rst index 86fed026..8332a6a4 100644 --- a/docs/persistence.rst +++ b/docs/persistence.rst @@ -113,7 +113,7 @@ objects having references to functions such as ``numpy.sqrt``. Command Line Interface ###################### -Skops has a command line interface to convert SciKit-Learn models persisted with +Skops has a command line interface to convert scikit-learn models persisted with ``Pickle`` to ``Skops`` files. To convert a file from the command line, use the ``skops convert`` entrypoint. From c38cb735133f7739e038da88e0e0f524bc898036 Mon Sep 17 00:00:00 2001 From: Erin Aho Date: Tue, 17 Jan 2023 11:18:56 +0000 Subject: [PATCH 29/38] Update docs/persistence.rst Co-authored-by: Benjamin Bossan --- docs/persistence.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/persistence.rst b/docs/persistence.rst index 8332a6a4..04f1234e 100644 --- a/docs/persistence.rst +++ b/docs/persistence.rst @@ -118,7 +118,7 @@ Skops has a command line interface to convert scikit-learn models persisted with To convert a file from the command line, use the ``skops convert`` entrypoint. -Below is an example call to convert a file ``my_model.pkl`` to ``new_model.skops``: +Below is an example call to convert a file ``my_model.pkl`` to ``my_model.skops``: .. code:: console skops convert my_model.pkl From 5233230806f4181633cea1a9264d7e43369d7833 Mon Sep 17 00:00:00 2001 From: Erin Aho Date: Tue, 17 Jan 2023 11:32:32 +0000 Subject: [PATCH 30/38] Apply suggestions from code review Co-authored-by: Benjamin Bossan --- docs/persistence.rst | 1 + skops/cli/_convert.py | 7 +++---- skops/cli/_utils.py | 2 +- skops/cli/entrypoint.py | 3 +-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/docs/persistence.rst b/docs/persistence.rst index 04f1234e..943a2196 100644 --- a/docs/persistence.rst +++ b/docs/persistence.rst @@ -121,6 +121,7 @@ To convert a file from the command line, use the ``skops convert`` entrypoint. Below is an example call to convert a file ``my_model.pkl`` to ``my_model.skops``: .. code:: console + skops convert my_model.pkl Further help for the different supported options can be found by calling diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index e68d2760..1e2095ee 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -11,9 +11,8 @@ from skops.io import dumps, get_untrusted_types -def _convert_file(input_file: os.PathLike, output_file: os.PathLike): - """ - Function that is called by ``skops convert`` entrypoint. +def _convert_file(input_file: os.PathLike, output_file: os.PathLike) -> None: + """Function that is called by ``skops convert`` entrypoint. Loads a pickle model from the input path, converts to skops format, and saves to output file. @@ -92,7 +91,7 @@ def format_parser( def main( parsed_args: argparse.Namespace, -): +) -> None: output_file = parsed_args.output_file input_file = parsed_args.input diff --git a/skops/cli/_utils.py b/skops/cli/_utils.py index 82a371ab..205b836a 100644 --- a/skops/cli/_utils.py +++ b/skops/cli/_utils.py @@ -1,7 +1,7 @@ import logging -def get_log_level(level: int = 0): +def get_log_level(level: int = 0) -> int: """Takes in verbosity from a CLI entrypoint (number of times -v specified), and sets the logger to the required log level""" diff --git a/skops/cli/entrypoint.py b/skops/cli/entrypoint.py index f2a7ac17..3e9acaf8 100644 --- a/skops/cli/entrypoint.py +++ b/skops/cli/entrypoint.py @@ -4,8 +4,7 @@ def main_cli(command_line_args=None): - """ - Main command line interface entrypoint for all command line Skops methods. + """Main command line interface entrypoint for all command line Skops methods. To add a new entrypoint: 1. Create a new method to call that accepts a namespace From 95a5ff3815e3e1d6b048f827cd9b7ed763524e3a Mon Sep 17 00:00:00 2001 From: = Date: Tue, 17 Jan 2023 11:41:40 +0000 Subject: [PATCH 31/38] Local changes --- skops/cli/_convert.py | 1 - 1 file changed, 1 deletion(-) diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index e68d2760..0c27eb59 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -43,7 +43,6 @@ def _convert_file(input_file: os.PathLike, output_file: os.PathLike): untrusted_str = ", ".join(untrusted_types) logging.warning( - "Unknown Types Detected! " f"While converting {model_name}, " "the following unknown types were found: " f"{untrusted_str}. " From 7e631783a2f2bf58cb39045ba674512f7073ac06 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 17 Jan 2023 12:02:16 +0000 Subject: [PATCH 32/38] Add test coverage for very verbose logging --- skops/cli/_utils.py | 2 +- skops/cli/tests/test_convert.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/skops/cli/_utils.py b/skops/cli/_utils.py index 205b836a..55173532 100644 --- a/skops/cli/_utils.py +++ b/skops/cli/_utils.py @@ -8,7 +8,7 @@ def get_log_level(level: int = 0) -> int: all_levels = [logging.WARNING, logging.INFO, logging.DEBUG] if level > len(all_levels): - level = len(all_levels) + level = len(all_levels) - 1 elif level < 0: level = 0 diff --git a/skops/cli/tests/test_convert.py b/skops/cli/tests/test_convert.py index 049d44b8..d7a3a69e 100644 --- a/skops/cli/tests/test_convert.py +++ b/skops/cli/tests/test_convert.py @@ -117,7 +117,7 @@ def test_with_output_dir_works_as_expected( ("--verbose", logging.INFO), ("-vv", logging.DEBUG), ("-v -v", logging.DEBUG), - ("-vvv", logging.DEBUG), + ("-vvvvv", logging.DEBUG), ("--verbose --verbose", logging.DEBUG), ], ) From 6e357705feb0d75cbb8c12000371427886d0a597 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 17 Jan 2023 12:10:25 +0000 Subject: [PATCH 33/38] Add comments to make entrypoint more clear --- skops/cli/entrypoint.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/skops/cli/entrypoint.py b/skops/cli/entrypoint.py index 3e9acaf8..c98ef8d3 100644 --- a/skops/cli/entrypoint.py +++ b/skops/cli/entrypoint.py @@ -25,7 +25,7 @@ def main_cli(command_line_args=None): ) # function_map should map a command to - # method: the command to call + # method: the command to call (gets set to default 'func') # format_parser: the function used to create a subparser for that command function_map = { "convert": { @@ -35,9 +35,13 @@ def main_cli(command_line_args=None): } for func_name, values in function_map.items(): + # Add subparser for each function in func map, + # and assigns default func to be "method" from function_map subparser = subparsers.add_parser(func_name) subparser.set_defaults(func=values["method"]) values["format_parser"](subparser) + # Parse arguments with arg parser for given function in function map, + # Then call the matching method in the function_map with the argument namespace args = entry_parser.parse_args(command_line_args) args.func(args) From 02c95ad014e5d6d0c84f2925a6a0aabc5295f8df Mon Sep 17 00:00:00 2001 From: = Date: Tue, 17 Jan 2023 12:14:20 +0000 Subject: [PATCH 34/38] Adjust test to be more clear --- skops/cli/tests/test_convert.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/skops/cli/tests/test_convert.py b/skops/cli/tests/test_convert.py index d7a3a69e..f7ee3a46 100644 --- a/skops/cli/tests/test_convert.py +++ b/skops/cli/tests/test_convert.py @@ -1,6 +1,6 @@ import logging import pathlib -import pickle as pkl +import pickle from unittest import mock import numpy as np @@ -37,12 +37,12 @@ def skops_path(self, tmp_path): @pytest.fixture def write_safe_file(self, pkl_path, safe_obj): with open(pkl_path, "wb") as f: - pkl.dump(safe_obj, f) + pickle.dump(safe_obj, f) @pytest.fixture def write_unsafe_file(self, pkl_path, unsafe_obj): with open(pkl_path, "wb") as f: - pkl.dump(unsafe_obj, f) + pickle.dump(unsafe_obj, f) def test_base_case_works_as_expected( self, pkl_path, tmp_path, skops_path, write_safe_file, safe_obj, caplog @@ -88,7 +88,7 @@ def test_base_works_as_expected(self, mock_convert: mock.MagicMock): @pytest.mark.parametrize( "input_path, output_file, expected_path", [ - ("abc.123", "a/b/c", "a/b/c"), + ("abc.123", "some/file/path.out", "some/file/path.out"), ("abc.123", None, pathlib.Path.cwd() / "abc.skops"), ], ids=["Given an output path", "No output path"], From c3491efe4005fed8f515538f435ddab1042067a5 Mon Sep 17 00:00:00 2001 From: = Date: Tue, 17 Jan 2023 12:42:17 +0000 Subject: [PATCH 35/38] Add example script to convert all pkl files in working dir --- docs/persistence.rst | 7 +++++++ skops/cli/_convert.py | 14 +++++++++----- skops/cli/tests/test_convert.py | 8 ++++++-- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/docs/persistence.rst b/docs/persistence.rst index 943a2196..473b9b6f 100644 --- a/docs/persistence.rst +++ b/docs/persistence.rst @@ -124,6 +124,13 @@ Below is an example call to convert a file ``my_model.pkl`` to ``my_model.skops` skops convert my_model.pkl +To convert multiple files, you can use bash commands to iterate the above call. +For example, to convert all ``.pkl`` flies in the current directory: + +.. code:: console + + for FILE in *.pkl; do skops convert FILE; done + Further help for the different supported options can be found by calling ``skops convert --help`` in a terminal. diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index 7f6708ad..946c8948 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -11,7 +11,11 @@ from skops.io import dumps, get_untrusted_types -def _convert_file(input_file: os.PathLike, output_file: os.PathLike) -> None: +def _convert_file( + input_file: os.PathLike, + output_file: os.PathLike, + logger: logging.Logger = logging.getLogger(), +) -> None: """Function that is called by ``skops convert`` entrypoint. Loads a pickle model from the input path, converts to skops format, and saves to @@ -28,7 +32,7 @@ def _convert_file(input_file: os.PathLike, output_file: os.PathLike) -> None: """ model_name = pathlib.Path(input_file).stem - logging.debug(f"Converting {model_name}") + logger.debug(f"Converting {model_name}") with open(input_file, "rb") as f: obj = pickle.load(f) @@ -37,11 +41,11 @@ def _convert_file(input_file: os.PathLike, output_file: os.PathLike) -> None: untrusted_types = get_untrusted_types(data=skops_dump) if not untrusted_types: - logging.info(f"No unknown types found in {model_name}.") + logger.info(f"No unknown types found in {model_name}.") else: untrusted_str = ", ".join(untrusted_types) - logging.warning( + logger.warning( f"While converting {model_name}, " "the following unknown types were found: " f"{untrusted_str}. " @@ -49,7 +53,7 @@ def _convert_file(input_file: os.PathLike, output_file: os.PathLike) -> None: ) with open(output_file, "wb") as out_file: - logging.debug(f"Writing to {output_file}") + logger.debug(f"Writing to {output_file}") out_file.write(skops_dump) diff --git a/skops/cli/tests/test_convert.py b/skops/cli/tests/test_convert.py index f7ee3a46..6395dd3d 100644 --- a/skops/cli/tests/test_convert.py +++ b/skops/cli/tests/test_convert.py @@ -47,10 +47,14 @@ def write_unsafe_file(self, pkl_path, unsafe_obj): def test_base_case_works_as_expected( self, pkl_path, tmp_path, skops_path, write_safe_file, safe_obj, caplog ): - _convert._convert_file(pkl_path, skops_path) + mock_logger = mock.MagicMock() + _convert._convert_file(pkl_path, skops_path, logger=mock_logger) persisted_obj = load(skops_path) assert np.array_equal(persisted_obj, safe_obj) - assert MockUnsafeType.__name__ not in caplog.text + + # Check no warnings or errors raised + mock_logger.warning.assert_not_called() + mock_logger.error.assert_not_called() def test_unsafe_case_works_as_expected( self, pkl_path, tmp_path, skops_path, write_unsafe_file, caplog From 7e072da43e96c432b2bec46443086ecd46d27677 Mon Sep 17 00:00:00 2001 From: Erin Aho Date: Wed, 18 Jan 2023 12:50:14 +0000 Subject: [PATCH 36/38] Update skops/cli/_convert.py to be more clear in warning message Co-authored-by: Benjamin Bossan --- skops/cli/_convert.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index 946c8948..d1f7a4e7 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -46,7 +46,7 @@ def _convert_file( untrusted_str = ", ".join(untrusted_types) logger.warning( - f"While converting {model_name}, " + f"While converting {input_file}, " "the following unknown types were found: " f"{untrusted_str}. " f"When loading {output_file}, add 'trusted=True' to the skops.load call. " From 799f91bcaf8f71befc17b022e510357d559f1792 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 18 Jan 2023 17:06:24 +0000 Subject: [PATCH 37/38] Update log message --- skops/cli/_convert.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index 946c8948..d8388d2d 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -98,7 +98,9 @@ def main( output_file = parsed_args.output_file input_file = parsed_args.input - logging.basicConfig(format="%(message)s", level=get_log_level(parsed_args.loglevel)) + logging.basicConfig( + format="%(levelname)-8s: %(message)s", level=get_log_level(parsed_args.loglevel) + ) if not output_file: # No filename provided, defaulting to base file path From 8ddc6b9cfb9115f7fbb4be06f8403a1beab55312 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 18 Jan 2023 17:10:18 +0000 Subject: [PATCH 38/38] Adjust warning message to not use Trusted=True --- skops/cli/_convert.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/skops/cli/_convert.py b/skops/cli/_convert.py index 4e8f0855..7d630c15 100644 --- a/skops/cli/_convert.py +++ b/skops/cli/_convert.py @@ -49,7 +49,8 @@ def _convert_file( f"While converting {input_file}, " "the following unknown types were found: " f"{untrusted_str}. " - f"When loading {output_file}, add 'trusted=True' to the skops.load call. " + f"When loading {output_file} with skops.load, these types must be " + "specified as 'trusted'" ) with open(output_file, "wb") as out_file: