From 9d9a453c921be91484c4426d1bda862baf69b493 Mon Sep 17 00:00:00 2001 From: "swapdewalkar@gmail.com" Date: Tue, 9 Apr 2024 02:00:49 +0530 Subject: [PATCH 1/5] No Output Requested while materiliazer should lead to error --- hamilton/driver.py | 2 ++ tests/test_end_to_end.py | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/hamilton/driver.py b/hamilton/driver.py index 1892d81d3..f8d757540 100644 --- a/hamilton/driver.py +++ b/hamilton/driver.py @@ -1454,6 +1454,8 @@ def materialize( materializer_vars = [] try: materializer_factories, extractor_factories = self._process_materializers(materializers) + if len(materializer_factories)==0: + raise ValueError("No output requested or materializer factories were provided.") function_graph = materialization.modify_graph( self.graph, materializer_factories, extractor_factories ) diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index a714e892a..9140d9534 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -353,6 +353,30 @@ def processed_data(input_data: dict) -> dict: assert json.load(f) == {"processed": True} +def test_no_materialize_failure(tmp_path_factory): + def processed_data(input_data: dict) -> dict: + data = input_data.copy() + data["processed"] = True + return data + + path_in = tmp_path_factory.mktemp("home") / "unprocessed_data.json" + path_out = tmp_path_factory.mktemp("home") / "processed_data.json" + + with open(path_in, "w") as f: + json.dump({"processed": False}, f) + + mod = ad_hoc_utils.create_temporary_module(processed_data) + + dr = driver.Driver({}, mod) + + with pytest.raises(ValueError): + dr.materialize( + from_.json(target="input_data", path=value(path_in)), + additional_vars=["processed_data"], + inputs={"output_path": str(path_out)}, + ) + + def test_driver_validate_with_overrides(): dr = ( driver.Builder() From 0e45ae08718306444df4334b574a9e38f1567767 Mon Sep 17 00:00:00 2001 From: "swapdewalkar@gmail.com" Date: Tue, 9 Apr 2024 02:10:50 +0530 Subject: [PATCH 2/5] fix black --- hamilton/driver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hamilton/driver.py b/hamilton/driver.py index f8d757540..3a71e6f1e 100644 --- a/hamilton/driver.py +++ b/hamilton/driver.py @@ -1454,7 +1454,7 @@ def materialize( materializer_vars = [] try: materializer_factories, extractor_factories = self._process_materializers(materializers) - if len(materializer_factories)==0: + if len(materializer_factories) == 0: raise ValueError("No output requested or materializer factories were provided.") function_graph = materialization.modify_graph( self.graph, materializer_factories, extractor_factories From ac247632e6a477c8cf3f1a1c455bee5385e754f1 Mon Sep 17 00:00:00 2001 From: "swapdewalkar@gmail.com" Date: Tue, 9 Apr 2024 20:16:08 +0530 Subject: [PATCH 3/5] Consider a additional_args as output as add unit tests --- hamilton/driver.py | 2 +- tests/test_end_to_end.py | 56 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/hamilton/driver.py b/hamilton/driver.py index 3a71e6f1e..36ff835fb 100644 --- a/hamilton/driver.py +++ b/hamilton/driver.py @@ -1454,7 +1454,7 @@ def materialize( materializer_vars = [] try: materializer_factories, extractor_factories = self._process_materializers(materializers) - if len(materializer_factories) == 0: + if len(materializer_factories) == len(final_vars) == 0: raise ValueError("No output requested or materializer factories were provided.") function_graph = materialization.modify_graph( self.graph, materializer_factories, extractor_factories diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index 9140d9534..1a65e39a6 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -353,6 +353,61 @@ def processed_data(input_data: dict) -> dict: assert json.load(f) == {"processed": True} +def test_materialize_and_loaders_end_to_end_without_additional_vars(tmp_path_factory): + def processed_data(input_data: dict) -> dict: + data = input_data.copy() + data["processed"] = True + return data + + path_in = tmp_path_factory.mktemp("home") / "unprocessed_data.json" + path_out = tmp_path_factory.mktemp("home") / "processed_data.json" + + with open(path_in, "w") as f: + json.dump({"processed": False}, f) + + mod = ad_hoc_utils.create_temporary_module(processed_data) + + dr = driver.Driver({}, mod) + + materialization_result, result = dr.materialize( + from_.json(target="input_data", path=value(path_in)), + to.json( + id="materializer", + dependencies=["processed_data"], + path=source("output_path"), + combine=JoinBuilder(), + ), + inputs={"output_path": str(path_out)}, + ) + assert "materializer" in materialization_result + + with open(path_out) as f: + assert json.load(f) == {"processed": True} + + +def test_materialize_and_loaders_end_to_end_without_to(tmp_path_factory): + def processed_data(input_data: dict) -> dict: + data = input_data.copy() + data["processed"] = True + return data + + path_in = tmp_path_factory.mktemp("home") / "unprocessed_data.json" + path_out = tmp_path_factory.mktemp("home") / "processed_data.json" + + with open(path_in, "w") as f: + json.dump({"processed": False}, f) + + mod = ad_hoc_utils.create_temporary_module(processed_data) + + dr = driver.Driver({}, mod) + + materialization_result, result = dr.materialize( + from_.json(target="input_data", path=value(path_in)), additional_vars=["processed_data"] + ) + assert result["processed_data"] == {"processed": True} + assert "materializer" not in materialization_result + + def test_no_materialize_failure(tmp_path_factory): def processed_data(input_data: dict) -> dict: data = input_data.copy() @@ -372,7 +427,6 @@ def processed_data(input_data: dict) -> dict: with pytest.raises(ValueError): dr.materialize( from_.json(target="input_data", path=value(path_in)), - additional_vars=["processed_data"], inputs={"output_path": str(path_out)}, ) From ccf964e977bc3c187c81d12adca0fed1bae0fb8d Mon Sep 17 00:00:00 2001 From: "swapdewalkar@gmail.com" Date: Tue, 9 Apr 2024 20:27:53 +0530 Subject: [PATCH 4/5] Remove used parameter --- tests/test_end_to_end.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index 1a65e39a6..b1f4585d9 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -392,7 +392,6 @@ def processed_data(input_data: dict) -> dict: return data path_in = tmp_path_factory.mktemp("home") / "unprocessed_data.json" - path_out = tmp_path_factory.mktemp("home") / "processed_data.json" with open(path_in, "w") as f: json.dump({"processed": False}, f) From bed8679c74ac3776ff594eca123ae44e5fd2b56e Mon Sep 17 00:00:00 2001 From: Stefan Krawczyk Date: Tue, 9 Apr 2024 13:21:58 -0700 Subject: [PATCH 5/5] Update hamilton/driver.py --- hamilton/driver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hamilton/driver.py b/hamilton/driver.py index 36ff835fb..19f6f20dd 100644 --- a/hamilton/driver.py +++ b/hamilton/driver.py @@ -1455,7 +1455,7 @@ def materialize( try: materializer_factories, extractor_factories = self._process_materializers(materializers) if len(materializer_factories) == len(final_vars) == 0: - raise ValueError("No output requested or materializer factories were provided.") + raise ValueError("No output requested. Please either pass in materializers that will save data, or pass in `additional_vars` to compute.") function_graph = materialization.modify_graph( self.graph, materializer_factories, extractor_factories )