From f5208aaac55cf7717e507988e0e733cf2a46e981 Mon Sep 17 00:00:00 2001 From: Maxime Liquet <35924738+maximlt@users.noreply.github.com> Date: Mon, 31 Jul 2023 16:19:50 +0200 Subject: [PATCH] `Path` parameter: raise if path not found on parameter instantiation and add `check_exists` attribute (#800) --- examples/user_guide/Parameter_Types.ipynb | 32 ++- param/__init__.py | 44 ++++- tests/testpathparam.py | 231 ++++++++++++++++++++-- 3 files changed, 275 insertions(+), 32 deletions(-) diff --git a/examples/user_guide/Parameter_Types.ipynb b/examples/user_guide/Parameter_Types.ipynb index 9f4462f50..9671648cc 100644 --- a/examples/user_guide/Parameter_Types.ipynb +++ b/examples/user_guide/Parameter_Types.ipynb @@ -700,7 +700,9 @@ "\n", "A Path can be set to a string specifying the path of a file or folder. In code, the string should be specified in POSIX (UNIX) style, using forward slashes / and starting from / if absolute or some other character if relative. When retrieved, the string will be in the format of the user's operating system. \n", "\n", - "Relative paths are converted to absolute paths by searching for a matching filename on the filesystem. If `search_paths` is provided and not empty, the folders in that list are searched for the given filename, in order, returning the absolute path for the first match found by appending the provided path to the search path. An IOError is raised if the file or folder is not found. If `search_paths` is empty (the default), the file or folder is expected to be in the current working directory.\n", + "Relative paths are converted to absolute paths by searching for a matching filename on the filesystem in the current working directory (obtained with `os.getcwd()`). If `search_paths` is provided and not empty, the folders in that list are searched for the given filename, in order, returning the absolute path for the first match found by appending the provided path to the search path. An IOError is raised if the file or folder is not found. If `search_paths` is empty (the default), the file or folder is expected to be in the current working directory.\n", + "\n", + "When `check_exists` is set to `False` (default is `True`) the provided path can optionally exist. This is for instance useful to declare an output file path that is meant to be created at a later stage in a process. In the default case the path must exist, on Parameter instantiation and setting.\n", "\n", "Either a file or a folder name is accepted by `param.Path`, while `param.Filename` accepts only file names and `param.Foldername` accepts only folder names." ] @@ -716,6 +718,7 @@ " p = param.Path('Parameter_Types.ipynb')\n", " f = param.Filename('Parameter_Types.ipynb')\n", " d = param.Foldername('lib', search_paths=['/','/usr','/share'])\n", + " o = param.Filename('output.csv', check_exists=False)\n", " \n", "p = P()\n", "p.p" @@ -728,7 +731,7 @@ "metadata": {}, "outputs": [], "source": [ - "p.p='/usr/lib'\n", + "p.p = '/usr/lib'\n", "p.p" ] }, @@ -750,7 +753,7 @@ "outputs": [], "source": [ "with param.exceptions_summarized():\n", - " p.f='/usr/lib'" + " p.f = '/usr/lib'" ] }, { @@ -774,6 +777,29 @@ " p.d = 'Parameter_Types.ipynb'" ] }, + { + "cell_type": "code", + "execution_count": null, + "id": "53225144", + "metadata": {}, + "outputs": [], + "source": [ + "p.o # the output file doesn't exist yet" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "863b9817", + "metadata": {}, + "outputs": [], + "source": [ + "with open(p.o, 'w') as f:\n", + " f.write('Param is awesome!')\n", + "\n", + "p.o # it now exists, getting its value resolve the full file path" + ] + }, { "cell_type": "markdown", "id": "573079ef", diff --git a/param/__init__.py b/param/__init__.py index 0d2dbd43f..8f1ec496b 100644 --- a/param/__init__.py +++ b/param/__init__.py @@ -2315,12 +2315,11 @@ def __call__(self,path="",**params): class Path(Parameter): - """ - Parameter that can be set to a string specifying the path of a file or folder. + """Parameter that can be set to a string specifying the path of a file or folder. The string should be specified in UNIX style, but it will be returned in the format of the user's operating system. Please use - the Filename or Foldername classes if you require discrimination + the Filename or Foldername Parameters if you require discrimination between the two possibilities. The specified path can be absolute, or relative to either: @@ -2332,26 +2331,42 @@ class Path(Parameter): * any of the paths searched by resolve_path() (if search_paths is None). + + Parameters + ---------- + search_paths : list, default=[os.getcwd()] + List of paths to search the path from + check_exists: boolean, default=True + If True (default) the path must exist on instantiation and set, + otherwise the path can optionally exist. """ - __slots__ = ['search_paths'] + __slots__ = ['search_paths', 'check_exists'] + + _slot_defaults = _dict_update( + Parameter._slot_defaults, check_exists=True, + ) @typing.overload def __init__( self, - default=None, *, search_paths=None, + default=None, *, search_paths=None, check_exists=True, allow_None=False, doc=None, label=None, precedence=None, instantiate=False, constant=False, readonly=False, pickle_default_value=True, per_instance=True ): ... @_deprecate_positional_args - def __init__(self, default=Undefined, *, search_paths=Undefined, **params): + def __init__(self, default=Undefined, *, search_paths=Undefined, check_exists=Undefined, **params): if search_paths is Undefined: search_paths = [] self.search_paths = search_paths + if check_exists is not Undefined and not isinstance(check_exists, bool): + raise ValueError("'check_exists' attribute value must be a boolean") + self.check_exists = check_exists super().__init__(default,**params) + self._validate(self.default) def _resolve(self, path): return resolve_path(path, path_to_file=None, search_paths=self.search_paths) @@ -2361,17 +2376,30 @@ def _validate(self, val): if not self.allow_None: raise ValueError(f'{_validate_error_prefix(self)} does not accept None') else: + if not isinstance(val, (str, pathlib.Path)): + raise ValueError(f'{_validate_error_prefix(self)} only take str or pathlib.Path types') try: self._resolve(val) except OSError as e: - Parameterized(name=f"{self.owner.name}.{self.name}").param.warning('%s',e.args[0]) + if self.check_exists: + raise OSError(e.args[0]) from None def __get__(self, obj, objtype): """ Return an absolute, normalized path (see resolve_path). """ raw_path = super().__get__(obj,objtype) - return None if raw_path is None else self._resolve(raw_path) + if raw_path is None: + path = None + else: + try: + path = self._resolve(raw_path) + except OSError: + if self.check_exists: + raise + else: + path = raw_path + return path def __getstate__(self): # don't want to pickle the search_paths diff --git a/tests/testpathparam.py b/tests/testpathparam.py index bbc8043ff..ff0d4c8b4 100644 --- a/tests/testpathparam.py +++ b/tests/testpathparam.py @@ -16,29 +16,42 @@ def setUp(self): super().setUp() tmpdir1 = tempfile.mkdtemp() + + self.curdir = os.getcwd() + # Chanding the directory to tmpdir1 to test that Path resolves relative + # paths to absolute paths automatically. + os.chdir(tmpdir1) + fa = os.path.join(tmpdir1, 'a.txt') fb = os.path.join(tmpdir1, 'b.txt') + fc = 'c.txt' open(fa, 'w').close() open(fb, 'w').close() + open(fc, 'w').close() self.tmpdir1 = tmpdir1 self.fa = fa self.fb = fb + self.fc = fc class P(param.Parameterized): a = param.Path() b = param.Path(self.fb) c = param.Path('a.txt', search_paths=[tmpdir1]) + d = param.Path(check_exists=False) + e = param.Path(self.fc, check_exists=False) + f = param.Path(self.fc) self.P = P def tearDown(self): - shutil.rmtree(self.tmpdir1) + os.chdir(self.curdir) def _check_defaults(self, p): assert p.default is None assert p.allow_None is True assert p.search_paths == [] + assert p.check_exists is True def test_defaults_class(self): class P(param.Parameterized): @@ -69,6 +82,10 @@ def test_no_path_inst(self): p = self.P() assert p.a is None + def test_unsupported_type(self): + with pytest.raises(ValueError): + param.Path(1) + def test_inst_with_path(self): p = self.P(a=self.fa) assert isinstance(p.a, str) @@ -85,6 +102,25 @@ def test_set_to_None_allowed(self): with pytest.raises(ValueError, match=re.escape(r"'Path' Parameter 'b' does not accept None")): p.b = None + def test_relative_cwd_class(self): + assert os.path.isabs(self.P.f) + + def test_relative_cwd_class_set(self): + self.P.a = self.fc + assert os.path.isabs(self.P.a) + + def test_relative_cwd_inst(self): + assert os.path.isabs(self.P().f) + + def test_relative_cwd_instantiation(self): + p = self.P(a=self.fc) + assert os.path.isabs(p.a) + + def test_relative_cwd_set(self): + p = self.P() + p.a = self.fc + assert os.path.isabs(p.a) + def test_search_paths(self): p = self.P() @@ -95,30 +131,101 @@ def test_search_paths(self): def test_inheritance_behavior(self): - # a = param.Path() - # b = param.Path(self.fb) - # c = param.Path('a.txt', search_paths=[tmpdir1]) + # Inheritance isn't working great with search_paths and this test + # isn't designed to be run from the tmpdir directory. + startd = os.getcwd() + try: + os.chdir(self.curdir) + # a = param.Path() + # b = param.Path(self.fb) + # c = param.Path('a.txt', search_paths=[tmpdir1]) + + class B(self.P): + a = param.Path() + b = param.Path() + c = param.Path() + + assert B.a is None + assert B.b == self.fb + # search_paths is empty instead of [tmpdir1] and getting c raises an error + assert B.param.c.search_paths == [] + with pytest.raises(OSError, match='Path a.txt was not found'): + assert B.c is None + + b = B() + + assert b.a is None + assert b.b == self.fb + + assert b.param.c.search_paths == [] + with pytest.raises(OSError, match='Path a.txt was not found'): + assert b.c is None + finally: + os.chdir(startd) + + def test_notfound_instantiation_raises_error(self): + with pytest.raises( + OSError, + match=r"Path file was not found in the following place\(s\): \['\S+(\\\\|/)non(\\\\|/)existing(\\\\|/)file'\]" + ): + param.Path('non/existing/file') + + def test_set_notfound_raises_error(self): + p = self.P() + with pytest.raises( + OSError, + match=r"Path file was not found in the following place\(s\): \['\S+(\\\\|/)non(\\\\|/)existing(\\\\|/)file'\]" + ): + p.a = 'non/existing/file' + + def test_set_notfound_class_raises_error(self): + with pytest.raises( + OSError, + match=r"Path file was not found in the following place\(s\): \['\S+(\\\\|/)non(\\\\|/)existing(\\\\|/)file'\]" + ): + self.P.a = 'non/existing/file' + + def test_nonexisting_unbound_no_error(self): + p = param.Path('non/existing/file', check_exists=False) + assert p.default == 'non/existing/file' + + def test_nonexisting_class_no_error(self): + self.P.d = 'non/existing/file' + assert self.P.d == 'non/existing/file' + + def test_nonexisting_instantiation_no_error(self): + p = self.P(d='non/existing/file') + assert p.d == 'non/existing/file' + + def test_nonexisting_set_no_error(self): + p = self.P() + p.d = 'non/existing/file' + assert p.d == 'non/existing/file' - class B(self.P): - a = param.Path() - b = param.Path() - c = param.Path() + def test_optionalexistence_unbound_no_error(self): + p = param.Path(self.fa, check_exists=False) + assert os.path.isabs(p.default) - assert B.a is None - assert B.b == self.fb - # search_paths is empty instead of [tmpdir1] and getting c raises an error - assert B.param.c.search_paths == [] - with pytest.raises(OSError, match='Path a.txt was not found'): - assert B.c is None + def test_optionalexistence_class_no_error(self): + assert os.path.isabs(self.P.e) + self.P.d = self.fc + assert os.path.isabs(self.P.d) - b = B() + def test_optionalexistence_instantiation_no_error(self): + p = self.P(d=self.fc) + assert os.path.isabs(p.d) - assert b.a is None - assert b.b == self.fb + def test_optionalexistence_set_no_error(self): + p = self.P() + p.d = self.fc + assert os.path.isabs(p.d) - assert b.param.c.search_paths == [] - with pytest.raises(OSError, match='Path a.txt was not found'): - assert b.c is None + def test_existence_bad_value(self): + with pytest.raises( + ValueError, + match="'check_exists' attribute value must be a boolean" + ): + param.Path(check_exists='wrong_option') class TestFilenameParameters(unittest.TestCase): @@ -140,6 +247,7 @@ class P(param.Parameterized): a = param.Filename() b = param.Filename(self.fb) c = param.Filename('a.txt', search_paths=[tmpdir1]) + d = param.Filename(check_exists=False) self.P = P @@ -150,6 +258,7 @@ def _check_defaults(self, p): assert p.default is None assert p.allow_None is True assert p.search_paths == [] + assert p.check_exists is True def test_defaults_class(self): class P(param.Parameterized): @@ -202,6 +311,45 @@ def test_search_paths(self): assert os.path.isabs(p.c) assert p.c == self.fa + def test_notfound_instantiation_raises_error(self): + with pytest.raises( + OSError, + match=r"File file was not found in the following place\(s\): \['\S+(\\\\|/)non(\\\\|/)existing(\\\\|/)file'\]" + ): + param.Filename('non/existing/file') + + def test_set_notfound_class_raises_error(self): + with pytest.raises( + OSError, + match=r"File file was not found in the following place\(s\): \['\S+(\\\\|/)non(\\\\|/)existing(\\\\|/)file'\]" + ): + self.P.a = 'non/existing/file' + + def test_set_notfound_raises_error(self): + p = self.P() + with pytest.raises( + OSError, + match=r"File file was not found in the following place\(s\): \['\S+(\\\\|/)non(\\\\|/)existing(\\\\|/)file'\]" + ): + p.a = 'non/existing/file' + + def test_nonexisting_unbound_no_error(self): + p = param.Filename('non/existing/file', check_exists=False) + assert p.default == 'non/existing/file' + + def test_nonexisting_class_no_error(self): + self.P.d = 'non/existing/file' + assert self.P.d == 'non/existing/file' + + def test_nonexisting_instantiation_no_error(self): + p = self.P(d='non/existing/file') + assert p.d == 'non/existing/file' + + def test_nonexisting_set_no_error(self): + p = self.P() + p.d = 'non/existing/file' + assert p.d == 'non/existing/file' + class TestFoldernameParameters(unittest.TestCase): @@ -218,7 +366,8 @@ def setUp(self): class P(param.Parameterized): a = param.Foldername() b = param.Foldername(tmpdir1) - c = param.Path('da', search_paths=[tmpdir1]) + c = param.Foldername('da', search_paths=[tmpdir1]) + d = param.Foldername(check_exists=False) self.P = P @@ -229,6 +378,7 @@ def _check_defaults(self, p): assert p.default is None assert p.allow_None is True assert p.search_paths == [] + assert p.check_exists is True def test_defaults_class(self): class P(param.Parameterized): @@ -281,3 +431,42 @@ def test_search_paths(self): assert os.path.isdir(p.c) assert os.path.isabs(p.c) assert p.c == self.da + + def test_notfound_instantiation_raises_error(self): + with pytest.raises( + OSError, + match=r"Folder folder was not found in the following place\(s\): \['\S+(\\\\|/)non(\\\\|/)existing(\\\\|/)folder'\]" + ): + param.Foldername('non/existing/folder') + + def test_set_notfound_raises_error(self): + p = self.P() + with pytest.raises( + OSError, + match=r"Folder folder was not found in the following place\(s\): \['\S+(\\\\|/)non(\\\\|/)existing(\\\\|/)folder'\]" + ): + p.a = 'non/existing/folder' + + def test_set_notfound_class_raises_error(self): + with pytest.raises( + OSError, + match=r"Folder folder was not found in the following place\(s\): \['\S+(\\\\|/)non(\\\\|/)existing(\\\\|/)folder'\]" + ): + self.P.a = 'non/existing/folder' + + def test_nonexisting_unbound_no_error(self): + p = param.Foldername('non/existing/folder', check_exists=False) + assert p.default == 'non/existing/folder' + + def test_nonexisting_class_no_error(self): + self.P.d = 'non/existing/folder' + assert self.P.d == 'non/existing/folder' + + def test_nonexisting_instantiation_no_error(self): + p = self.P(d='non/existing/folder') + assert p.d == 'non/existing/folder' + + def test_nonexisting_set_no_error(self): + p = self.P() + p.d = 'non/existing/folder' + assert p.d == 'non/existing/folder'