Skip to content

Add join_mappings#841

Merged
bbayles merged 4 commits intomasterfrom
join_map
May 22, 2024
Merged

Add join_mappings#841
bbayles merged 4 commits intomasterfrom
join_map

Conversation

@bbayles
Copy link
Copy Markdown
Collaborator

@bbayles bbayles commented May 20, 2024

Re: #839, this PR adds join_mappings.

@bbayles bbayles changed the title Add join_mapping Add join_mappings May 20, 2024
def powerset_of_sets(iterable: Iterable[_T]) -> Iterator[set[_T]]: ...
def join_mappings(
**field_to_map: dict[Any, dict[Any, Any]]
) -> dict[Any, dict[Any, Any]]: ...
Copy link
Copy Markdown

@NeilGirdhar NeilGirdhar May 20, 2024

Choose a reason for hiding this comment

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

This annotation seems to have one too many levels of dict on the parameter (your example won't even pass). I would do something like:

def join_mappings(
    **field_to_map: Mapping[T, V]
) -> dict[T, dict[str, V]]: ...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Left over from the earlier implementation - thanks


>>> user_scores = {'elliot': 50, 'claris': 60}
>>> user_times = {'elliot': 30, 'claris': 40}
>>> dict(join_mappings(score=user_scores, time=user_times))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why make another copy of the return value, which is necessarily a copy already?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Left over from the earlier implementation - thanks

yield from starmap(set().union, combinations(sets, r))


def join_mappings(**field_to_map):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cool design of joining them by name instead of tuple-index!

@bbayles bbayles merged commit 86c21de into master May 22, 2024
@bbayles bbayles deleted the join_map branch May 22, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants