Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP for injected kwargs #315

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions hamilton/function_modifiers/adapters.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import enum
import inspect
import typing
from typing import Any, Callable, Collection, Dict, List, Tuple, Type
Expand All @@ -18,6 +19,14 @@
from hamilton.registry import LOADER_REGISTRY, SAVER_REGISTRY


class InjectedKwargs(enum.Enum):
"""These are special kwargs for data adapters that are declared and automatically injected by Hamilton.
These can"""

node_name = "__node_name"
node_tags = "__node_tags"
Comment on lines +26 to +27
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we need a prefix, since this isn't "this" node's name, but the one it depends on / wraps.

Also in the case of multiple outputs -> combiner -> saver. We'd likely want this to be an ordered list for names, and tags to be a dictionary of name -> tags?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this PR is very much in flux -- I've been thinking it through today and need to solve those edge cases.



class AdapterFactory:
"""Factory for data loaders. This handles the fact that we pass in source(...) and value(...)
parameters to the data loaders."""
Expand Down Expand Up @@ -60,6 +69,29 @@ def validate(self):
f"Extra parameters for loader: {self.adapter_cls} {extra_params}"
)

def resolve_injected_kwargs(self, node_: node.Node) -> Dict[str, Any]:
"""Resolves additional keyword arguments from the on which this operates
(passed into or extracted from). We may change how this works in the future, should
data loaders need metadata, but for now we'll be putting this in one function and
it'll be called by the data saver.

:param node_:
:return:
"""
possible_args = {
InjectedKwargs.node_name.value: node_.name,
InjectedKwargs.node_tags.value: node_.tags,
}
out = {}
declared_arguments = {
**self.adapter_cls.get_optional_arguments(),
**self.adapter_cls.get_required_arguments(),
}
for item in declared_arguments:
if item in declared_arguments:
out[item] = possible_args[item]
return out

def create_loader(self, **resolved_kwargs: Any) -> DataLoader:
if not self.adapter_cls.can_load():
raise InvalidDecoratorException(f"Adapter {self.adapter_cls} cannot load data.")
Expand Down Expand Up @@ -473,6 +505,8 @@ def create_saver_node(

adapter_factory = AdapterFactory(saver_cls, **self.kwargs)
dependencies, resolved_kwargs = resolve_kwargs(self.kwargs)
injected_kwargs = adapter_factory.resolve_injected_kwargs(node_)
resolved_kwargs.update(injected_kwargs)
dependencies_inverted = {v: k for k, v in dependencies.items()}

def save_data(
Expand Down
2 changes: 1 addition & 1 deletion hamilton/version.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
VERSION = (1, 27, 2)
VERSION = (1, 27, 3)