From 77972be11220b384e7cbb014dc269ab327ab7cdb Mon Sep 17 00:00:00 2001 From: gabrieldemarmiesse Date: Fri, 2 Aug 2019 14:39:50 +0200 Subject: [PATCH 1/6] Using pathlib to simplify is_local_source. --- sacred/dependencies.py | 12 +++++++----- sacred/utils.py | 7 ------- tests/test_utils.py | 20 +------------------- 3 files changed, 8 insertions(+), 31 deletions(-) diff --git a/sacred/dependencies.py b/sacred/dependencies.py index 02f7b453..d51f1fc0 100644 --- a/sacred/dependencies.py +++ b/sacred/dependencies.py @@ -6,7 +6,7 @@ import os.path import re import sys -import pathlib +from pathlib import Path import pkg_resources @@ -260,7 +260,7 @@ def create(cls, mod): def convert_path_to_module_parts(path): """Convert path to a python file into list of module names.""" - module_parts = list(pathlib.Path(path).parts) + module_parts = list(path.parts) if module_parts[-1] in ['__init__.py', '__init__.pyc']: # remove trailing __init__.py module_parts = module_parts[:-1] @@ -294,9 +294,11 @@ def is_local_source(filename, modname, experiment_path): True if the module was imported locally from (a subdir of) the experiment_path, and False otherwise. """ - if not is_subdir(filename, experiment_path): + filename = Path(filename).resolve() + experiment_path = Path(experiment_path).resolve() + if experiment_path not in filename.parents: return False - rel_path = os.path.relpath(filename, experiment_path) + rel_path = filename.relative_to(experiment_path) path_parts = convert_path_to_module_parts(rel_path) mod_parts = modname.split('.') @@ -304,7 +306,7 @@ def is_local_source(filename, modname, experiment_path): return True if len(path_parts) > len(mod_parts): return False - abs_path_parts = convert_path_to_module_parts(os.path.abspath(filename)) + abs_path_parts = convert_path_to_module_parts(filename) return all([p == m for p, m in zip(reversed(abs_path_parts), reversed(mod_parts))]) diff --git a/sacred/utils.py b/sacred/utils.py index 2ec16b7e..f7124550 100755 --- a/sacred/utils.py +++ b/sacred/utils.py @@ -547,13 +547,6 @@ def filtered_traceback_format(tb_exception, chain=True): yield from tb_exception.format_exception_only() -def is_subdir(path, directory): - path = os.path.abspath(os.path.realpath(path)) + os.sep - directory = os.path.abspath(os.path.realpath(directory)) + os.sep - - return path.startswith(directory) - - # noinspection PyUnusedLocal @wrapt.decorator def optional_kwargs_decorator(wrapped, instance=None, args=None, kwargs=None): diff --git a/tests/test_utils.py b/tests/test_utils.py index 6273b275..98400d74 100755 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -4,7 +4,7 @@ import pytest from sacred.utils import (PATHCHANGE, convert_to_nested_dict, - get_by_dotted_path, is_prefix, is_subdir, + get_by_dotted_path, is_prefix, iter_path_splits, iter_prefixes, iterate_flattened, iterate_flattened_separately, join_paths, recursive_update, set_by_dotted_path, get_inheritors, @@ -101,24 +101,6 @@ def test_convert_to_nested_dict_nested(): {'a': {'b': {'foo': {'bar': 8, 'baz': 7}}}} -@pytest.mark.parametrize('path,parent,expected', [ - ('/var/test2', '/var/test', False), - ('/var/test', '/var/test2', False), - ('var/test2', 'var/test', False), - ('var/test', 'var/test2', False), - ('/var/test/sub', '/var/test', True), - ('/var/test', '/var/test/sub', False), - ('var/test/sub', 'var/test', True), - ('var/test', 'var/test', True), - ('var/test', 'var/test/fake_sub/..', True), - ('var/test/sub/sub2/sub3/../..', 'var/test', True), - ('var/test/sub', 'var/test/fake_sub/..', True), - ('var/test', 'var/test/sub', False) -]) -def test_is_subdirectory(path, parent, expected): - assert is_subdir(path, parent) == expected - - def test_get_inheritors(): class A: pass From 4b948a3c4971ed168846d3eaa38a63ce98aa483c Mon Sep 17 00:00:00 2001 From: gabrieldemarmiesse Date: Fri, 2 Aug 2019 14:58:00 +0200 Subject: [PATCH 2/6] Fixed import. --- sacred/dependencies.py | 2 +- sacred/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sacred/dependencies.py b/sacred/dependencies.py index d51f1fc0..320809b0 100644 --- a/sacred/dependencies.py +++ b/sacred/dependencies.py @@ -12,7 +12,7 @@ import sacred.optional as opt from sacred import SETTINGS -from sacred.utils import is_subdir, iter_prefixes +from sacred.utils import iter_prefixes MB = 1048576 MODULE_BLACKLIST = set(sys.builtin_module_names) diff --git a/sacred/utils.py b/sacred/utils.py index f7124550..7020c53c 100755 --- a/sacred/utils.py +++ b/sacred/utils.py @@ -27,7 +27,7 @@ "set_by_dotted_path", "get_by_dotted_path", "iter_path_splits", "iter_prefixes", "join_paths", "is_prefix", "convert_to_nested_dict", "convert_camel_case_to_snake_case", - "print_filtered_stacktrace", "is_subdir", + "print_filtered_stacktrace", "optional_kwargs_decorator", "get_inheritors", "apply_backspaces_and_linefeeds", "rel_path", "IntervalTimer", "PathType"] From 833ff036ca49f02dbb7e47e4fc856774bd8c2ba9 Mon Sep 17 00:00:00 2001 From: gabrieldemarmiesse Date: Fri, 2 Aug 2019 15:14:24 +0200 Subject: [PATCH 3/6] Backport for py 3.5 --- sacred/dependencies.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/sacred/dependencies.py b/sacred/dependencies.py index 320809b0..c335c24e 100644 --- a/sacred/dependencies.py +++ b/sacred/dependencies.py @@ -270,6 +270,13 @@ def convert_path_to_module_parts(path): return module_parts +def resolve_not_strict(path): + """Backport of the strict flag in Path.resolve()""" + if sys.version_info < (3, 6): + return Path(os.path.abspath(os.path.realpath(path))) + else: + return Path(path).resolve() + def is_local_source(filename, modname, experiment_path): """Check if a module comes from the given experiment path. @@ -294,8 +301,8 @@ def is_local_source(filename, modname, experiment_path): True if the module was imported locally from (a subdir of) the experiment_path, and False otherwise. """ - filename = Path(filename).resolve() - experiment_path = Path(experiment_path).resolve() + filename = resolve_not_strict(filename) + experiment_path = resolve_not_strict(experiment_path) if experiment_path not in filename.parents: return False rel_path = filename.relative_to(experiment_path) From 262e5d0df65f5812173b708b5a399f239f5746fe Mon Sep 17 00:00:00 2001 From: gabrieldemarmiesse Date: Fri, 2 Aug 2019 15:14:47 +0200 Subject: [PATCH 4/6] pep8 --- sacred/dependencies.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sacred/dependencies.py b/sacred/dependencies.py index c335c24e..71e0d83a 100644 --- a/sacred/dependencies.py +++ b/sacred/dependencies.py @@ -277,6 +277,7 @@ def resolve_not_strict(path): else: return Path(path).resolve() + def is_local_source(filename, modname, experiment_path): """Check if a module comes from the given experiment path. From bc9430931631326098b9a5866d542e26752eba00 Mon Sep 17 00:00:00 2001 From: gabrieldemarmiesse Date: Fri, 2 Aug 2019 15:31:44 +0200 Subject: [PATCH 5/6] Removed unused import. --- sacred/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sacred/utils.py b/sacred/utils.py index 7020c53c..631897f0 100755 --- a/sacred/utils.py +++ b/sacred/utils.py @@ -6,7 +6,6 @@ import importlib import inspect import logging -import os.path import pkgutil import re import shlex From 43ef7cb1a720409d7b65f8f09b1b48b7354f76cf Mon Sep 17 00:00:00 2001 From: gabrieldemarmiesse Date: Fri, 2 Aug 2019 17:08:07 +0200 Subject: [PATCH 6/6] Removed the .resolve() --- sacred/dependencies.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/sacred/dependencies.py b/sacred/dependencies.py index 71e0d83a..a4bba2b9 100644 --- a/sacred/dependencies.py +++ b/sacred/dependencies.py @@ -270,14 +270,6 @@ def convert_path_to_module_parts(path): return module_parts -def resolve_not_strict(path): - """Backport of the strict flag in Path.resolve()""" - if sys.version_info < (3, 6): - return Path(os.path.abspath(os.path.realpath(path))) - else: - return Path(path).resolve() - - def is_local_source(filename, modname, experiment_path): """Check if a module comes from the given experiment path. @@ -302,8 +294,8 @@ def is_local_source(filename, modname, experiment_path): True if the module was imported locally from (a subdir of) the experiment_path, and False otherwise. """ - filename = resolve_not_strict(filename) - experiment_path = resolve_not_strict(experiment_path) + filename = Path(os.path.abspath(os.path.realpath(filename))) + experiment_path = Path(os.path.abspath(os.path.realpath(experiment_path))) if experiment_path not in filename.parents: return False rel_path = filename.relative_to(experiment_path)