Adds group feature for injecting kwargs

[elijahbenizzy]

We were doing this in the replacement function. Looks like everythin ghas to go in the replacement function, which makes things slightly trickier, but at least we can centralize the logic in that function.

[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.

[skrawcz]

Part of me wants the grouped dependency to be a dictionary (or list of tuples) and not a straight list. Why? So that someone could do this and get the correct behavior:

@inject(group(source("foo"), source("bar"), source("baz")))
def my_df(series_list: List[pd.Series]) -> pd.DataFrame:
    """One would likely expect the series to be named and thus the output to be "foo", "bar", "baz"... but it's not guaranteed."""
    return pd.concat(series_list)

@inject(group(source("foo"), source("bar"), source("baz")))
def my_df(series_list: List[Tuple[str, pd.Series]]) -> pd.DataFrame:
    """This would be guaranteed to work"""
    return pd.DataFrame(dict(series_list))

@inject(group(source("foo"), source("bar"), source("baz")))
def my_df(series_dict: Dict[str, pd.Series]) -> pd.DataFrame:
    """This would be guaranteed to work"""
    return pd.DataFrame(series_dict)

Or maybe we can do all the above and know what to do based on the parameter type annotation?

[skrawcz]

Otherwise @elijahbenizzy should someone be able to do this:

@inject(group(source("foo"), source("foo"), source("foo")))
def my_df(series_list: List[pd.Series]) -> pd.DataFrame:
    """Should we have three? or just one?"""
    return sum(series_list)

constant = pd.Series([1,2,3,4])
@inject(group(source("foo"), source("boo"), source("baz"), value(constant)))
def my_df(series_list: List[pd.Series]) -> pd.DataFrame:
    """Are values allowed?"""
    return sum(series_list)

[elijahbenizzy]

Otherwise @elijahbenizzy should someone be able to do this:

@inject(group(source("foo"), source("foo"), source("foo")))
def my_df(series_list: List[pd.Series]) -> pd.DataFrame:
    """Should we have three? or just one?"""
    return sum(series_list)

Ughh, yeah, currently this is broken but not for great reasons. Need to fix + add tests. Was thinking of releasing then fixing -- as this is a weird case. IMO the strangest case that will currently break is:

@Inject(group(source("foo"), source("boo"), source("baz")))
def my_df(series_list: List[pd.Series], foo: pd.Series) -> pd.DataFrame:
"""Are values allowed?"""
return sum(series_list)

This is busted due to the popping of kwargs when we parse

constant = pd.Series([1,2,3,4])
@Inject(group(source("foo"), source("boo"), source("baz"), value(constant)))
def my_df(series_list: List[pd.Series]) -> pd.DataFrame:
"""Are values allowed?"""
return sum(series_list)

This is fine by me -- it'll just work I think?

[skrawcz]

Otherwise @elijahbenizzy should someone be able to do this:

@inject(group(source("foo"), source("foo"), source("foo")))
def my_df(series_list: List[pd.Series]) -> pd.DataFrame:
    """Should we have three? or just one?"""
    return sum(series_list)

Ughh, yeah, currently this is broken but not for great reasons. Need to fix + add tests. Was thinking of releasing then fixing -- as this is a weird case.

I would argue against allowing duplicates, since you can achieve that another way more clearly.

IMO the strangest case that will currently break is:

@Inject(group(source("foo"), source("boo"), source("baz")))
def my_df(series_list: List[pd.Series], foo: pd.Series) -> pd.DataFrame:
"""Are values allowed?"""
return sum(series_list)

This is busted due to the popping of kwargs when we parse

constant = pd.Series([1,2,3,4])
@Inject(group(source("foo"), source("boo"), source("baz"), value(constant)))
def my_df(series_list: List[pd.Series]) -> pd.DataFrame:
"""Are values allowed?"""
return sum(series_list)

This is fine by me -- it'll just work I think?

I don't think we want that to work TBH. What's the "name" of the constant?

[elijahbenizzy]

Part of me wants the grouped dependency to be a dictionary (or list of tuples) and not a straight list. Why? So that someone could do this and get the correct behavior:

@inject(group(source("foo"), source("bar"), source("baz")))
def my_df(series_list: List[pd.Series]) -> pd.DataFrame:
    """One would likely expect the series to be named and thus the output to be "foo", "bar", "baz"... but it's not guaranteed."""
    return pd.concat(series_list)

@inject(group(source("foo"), source("bar"), source("baz")))
def my_df(series_list: List[Tuple[str, pd.Series]]) -> pd.DataFrame:
    """This would be guaranteed to work"""
    return pd.DataFrame(dict(series_list))

@inject(group(source("foo"), source("bar"), source("baz")))
def my_df(series_dict: Dict[str, pd.Series]) -> pd.DataFrame:
    """This would be guaranteed to work"""
    return pd.DataFrame(series_dict)

Or maybe we can do all the above and know what to do based on the parameter type annotation?

Yeah, so I think we should allow it to be a dict depending on the annotation -- it should be pretty easy. That said, we could just as well do a grouped_dict as well as grouped_list, so you could have to annotate it in the right way...

[elijahbenizzy]

Merging this to consolidate PRs -- these two were using similar pieces so I wanted to merge them together. See https://github.com/DAGWorks-Inc/hamilton/tree/configure-everything for continuation.