Skip to content

Commit

Permalink
Fix getdeps issue (#262)
Browse files Browse the repository at this point in the history
* update handle_deps to avoid defined multiple times issue

* add named getdeps test

* append instead of +=

* bump znjson version
  • Loading branch information
PythonFZ authored Mar 21, 2022
1 parent c5e3a96 commit 6f459d6
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 13 deletions.
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
dvc
PyYAML
znjson>=0.0.5
znjson>=0.1.2
dot4dict
37 changes: 37 additions & 0 deletions tests/integration_tests/test_node_to_node_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,40 @@ def test_getdeps_property(proj_path, load):
subprocess.check_call(["dvc", "repro"])

assert DependenciesCollector.load().dependencies == "muspI meroL"


class DefaultDependencyNodeTwo(Node):
deps1: ZnOuts = zn.deps()
deps2: ZnOuts = zn.deps()
result = zn.outs()

def run(self):
self.result = (self.deps1, self.deps2)


def test_getdeps_double(proj_path):
"""Test that getdeps twice in a single node to the same dependency does work"""
ZnOuts().write_graph()

deps = getdeps(ZnOuts.load(), "data")

DefaultDependencyNodeTwo(deps1=deps, deps2=deps).write_graph()

subprocess.check_call(["dvc", "repro"])

assert DefaultDependencyNodeTwo.load().result == ["Lorem Ipsum", "Lorem Ipsum"]


def test_getdeps_double_named(proj_path):
"""Test that getdeps twice in a single node to the same dependency does work"""
ZnOuts(name="outs1").write_graph()
ZnOuts(name="outs2").write_graph()

deps1 = getdeps(ZnOuts.load(name="outs1"), "data")
deps2 = getdeps(ZnOuts.load(name="outs2"), "data")

DefaultDependencyNodeTwo(deps1=deps1, deps2=deps2).write_graph()

subprocess.check_call(["dvc", "repro"])

assert DefaultDependencyNodeTwo.load().result == ["Lorem Ipsum", "Lorem Ipsum"]
8 changes: 4 additions & 4 deletions tests/unit_tests/core/test_dvcgraph.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,16 @@ def test_get_dvc_arguments():


def test_handle_deps():
assert handle_deps(["a.dat", "b.dat"]) == ["--deps", "a.dat", "--deps", "b.dat"]
assert handle_deps(["a.dat", "b.dat"]) == ["a.dat", "b.dat"]


def test_handle_deps_posix():
"""Change the path to posix for e.g. Windows users"""
# TODO this does not really test anything, but I don't know how to fix that,
# because converting windows path to posix under linux does not work natively
assert handle_deps(r"src\file.txt") in [
["--deps", "src/file.txt"],
["--deps", r"src\file.txt"],
["src/file.txt"],
[r"src\file.txt"],
]


Expand Down Expand Up @@ -77,7 +77,7 @@ def test_handle_deps_unknown():


def test_handle_deps_node():
assert handle_deps(ExampleDVCOutsNode()) == ["--deps", "example.dat"]
assert handle_deps(ExampleDVCOutsNode()) == ["example.dat"]


class ExampleAffectedFiles(GraphWriter):
Expand Down
20 changes: 12 additions & 8 deletions zntrack/core/dvcgraph.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
log = logging.getLogger(__name__)


def handle_deps(value) -> list:
def handle_deps(value) -> typing.List[str]:
"""Find all dependencies of value
Parameters
Expand All @@ -24,25 +24,25 @@ def handle_deps(value) -> list:
Returns
-------
list:
A list of strings like ["--deps", "<path>", --deps, "<path>", ...]
A list dependency files
"""
script = []
deps_files: typing.List[str] = []
if isinstance(value, (list, tuple)):
for lst_val in value:
script += handle_deps(lst_val)
deps_files += handle_deps(lst_val)
else:
if isinstance(value, (GraphWriter, NodeAttribute)):
for file in value.affected_files:
script += ["--deps", pathlib.Path(file).as_posix()]
deps_files.append(pathlib.Path(file).as_posix())
elif isinstance(value, (str, pathlib.Path)):
script += ["--deps", pathlib.Path(value).as_posix()]
deps_files.append(pathlib.Path(value).as_posix())
elif value is None:
pass
else:
raise ValueError(f"Type {type(value)} ({value}) is not supported!")

return script
return deps_files


@dataclasses.dataclass
Expand Down Expand Up @@ -355,6 +355,7 @@ def write_graph(
self.convert_notebook(nb_name)

custom_args = []
dependencies = []
# Handle Parameter
params_list = filter_ZnTrackOption(
data=self._descriptor_list, cls=self, zn_type=[utils.ZnTypes.PARAMS]
Expand Down Expand Up @@ -382,7 +383,10 @@ def write_graph(
)
elif option.zn_type == utils.ZnTypes.DEPS:
value = getattr(self, option.name)
custom_args += handle_deps(value)
dependencies += handle_deps(value)

for dependency in set(dependencies):
custom_args += ["--deps", dependency]

for pair in zn_options_set:
custom_args += pair
Expand Down

0 comments on commit 6f459d6

Please sign in to comment.