Configuration-driven DAG shaping + grouping #108

elijahbenizzy · 2023-03-09T18:21:50Z

[Short description explaining the high-level reason for the pull request]

Changes

How I tested this

Notes

Checklist

PR has an informative and human-readable title (this will be pulled into the release notes)
Changes are limited to a single goal (no scope creep)
Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
Any change in functionality is tested
New functions are documented (with a description, list of inputs, and expected output)
Placeholder code is flagged / future TODOs are captured in comments
Project documentation has been updated if adding/changing functionality.

tests/test_end_to_end.py

tests/resources/dynamic_config.py

hamilton/function_modifiers/delayed.py

skrawcz · 2023-03-12T04:55:14Z

tests/resources/dynamic_config.py

+@dynamic(
+    lambda columns_to_sum_map: parameterize(
+        **{
+            key: {"col_1": source(value[0]), "col_2": source(value[1])}
+            for key, value in columns_to_sum_map.items()
+        }
+    ),
+    at=ResolveAt.COMPILE,
+)
+def generic_summation(col_1: pd.Series, col_2: pd.Series) -> pd.Series:
+    return col_1 + col_2


@config.inject( lambda columns_to_sum_map: parameterize( **{ key: {"col_1": source(value[0]), "col_2": source(value[1])} for key, value in columns_to_sum_map.items() } ), at=ResolveAt.COMPILE, ) def generic_summation(col_1: pd.Series, col_2: pd.Series) -> pd.Series: return col_1 + col_2

I think that reads better...

Yeah that's not bad

OK, so I don't actually like putting this under config now that I think about it -- its pretty confusing. Specifically, we're not actually injecting anything, we're using it to delay evaluation. Furthermore, it doesn't actually have anything to do with the other config decorators, which people already have a tough time understanding. Its also pretty meta, so I want it kept on its own.

I'm thinking @delay_resolution(until=CONFIG_AVAILABLE, lambda ...). That's very accurate what it does, lives separately, and it nice and readable.

@resolve( when=CONFIG_AVAILABLE, decorate_with=lambda: conf_var_1 -> ...):

See #109 Note this makes the following decisions: 1. Chooses to use the generic `@dynamic` decorator 2. Buries it under a power-user mode 3. Uses the parameter names of the labmda/function to draw from the config We also Deprecate dynamic_transform This was an old way of doing the same thing. This was never accessible to users, as it reuqired implementing a subclass and konwing some of hamilton's internals. delay_resolution solves the same problem, and gives users access to any of the current decorators.

@source

A few things we need to iron out, but this allows group() in addition to @source and @value.

This is a little hacky but the API is solid. Basically this is a parameterized of 1 parameterization. It goes very well with config-driven pipelines, allowing you to group a set of functions to do stuff with.

There is a bit of duplicated code here, but its well-tested and solves a common problem. At some point we'll clean it up when we rewrite decorators, but for now this is OK. This allows mapping between the following: - group(*args) -> List[...] - group(**kwargs) -> Dict[str, ...] Anything else is not supported yet.

elijahbenizzy force-pushed the configure-everything branch 9 times, most recently from a4eb9fd to 4b4f996 Compare March 11, 2023 02:36