Figure out pattern to reduce visual overhead in code sections that leverage functional utilities and end up requiring large numbers of call arguments.

[pipermerriam]

What is wrong

The eth2 stuff has been implemented using a highly functional approach, comprised of many smaller utility functions that are pieced together in various ways to create the overall functionality. Many of these functions take 5 or more arguments and have reasonably long names.

An extreme case of this can be seen quite clearly in #273 in the eth2.beacon.state_machines.forks.serenity.epoch_processing.process_rewards_and_penalties function.

This problem seems to manifest itself in two flavors.

1. functions that need multiple values that can be sourced from a config object
2. functions that need data that has been computed in the local context.

Idea for how to fix it.

The short and high level description of a possible solution is to provide an API to easily partial these functions such that some arguments can be provided separately from whatever arguments are more dynamic and relevant to the local context.

For example.

# somewhere early on we have a block of these statements that can largely be visually ignored
do_some_fancy_thing = _do_some_fancy_thing.partial(state, config)

# then later in the "meat" of the execution
the_result = do_some_fancy_thing(
    important_locally_computed_data,
    other_local_data,
)

A good solution to this does two things.

1. concise API for currying/partial-ing the function with the arguments that are available early on.
2. ability to reduce the number of arguments that need to be provided at the point where the function is actually called.

The original raw idea

This was originally posted as a comment here: #228 (comment)

I think it is good that each of these nice functional utilities takes only the set of config values that it needs. This helps make it easy to see what config values each function depends on and keeps the implementations decoupled from the config object.

however, it's also really unfortunate how verbose some of this code ends up because you end up having to explicitly provide each parameter...

So I was thinking about an API that allowed you to take a config and auto-magically partial out all of these helper functions. Here's some pseudo code to illustrate.

class BeaconHelperAPI:
    def __init__(self, config):
        # option 1: do it dynamically in constructor (con: hard to maintain type safety)
        self.get_attesting_validator_indices = functools.partial(
            get_attesting_validator_indices,
            epoch_length=config.EPOCH_LENGTH ,
            ...
        )

    def get_attesting_validator_indices(state, attestations, shard, shard_block_root):
        # option 2: mirror the API (con: heavy on boilerplate, duplicates all function signatures)
        return get_attesting_validator_indices(..., epoch_length=self.config.EPOCH_LENGTH, ...)

I'm sure there are some other options for how it could be implemented as well. Here's an overly clever one that could be really good if done well but also might be too clever and end up bad.

class Wrapper:
    def __init__(self, fn):
        self.fn = fn
    def __call__(self, *args, **kwargs):
         return fn(*args, **kwargs)
    def partial(self, config):
         kwargs = {kw: getattr(config, kw.upper()) for kw in keywords_to_partial}
         return functools.partial(self.fn, **kwargs)

def configurable(*keywords_to_partial):
    def outer(fn):
        return Wrapper(fn)
    return outer

@configurable('epoch_length', 'target_committee_size', 'shard_count')
def get_attesting_validator_indices(stat, attestations, shard, shard_block_root, epoch_length, target_committee_size, shard_count):
    ...

# using it in code
get_attestating_validator_indices.partial(config)(state, attestations, shard, shard_block_root)

Anyways, some ideas on how you might make calling these helpers a little less cumbersome, but I'd advocate for ensuring that whatever solution you end up using, that you ensure that you maintain type safety.

cc @jannikluhn @hwwhww @ralexstokes (and anyone else I'm forgetting).

[hwwhww]

Whoops, I guess I should not have redirected it to #146 (comment). 😅

Copy-pasted + edited my previous comment:
#146 (comment)

1. I amended the Wrapper pseudo code to runnable code:

class Wrapper:
    def __init__(self, fn, keywords_to_partial):
        self.fn = fn
        self.keywords_to_partial = keywords_to_partial

    def __call__(self, *args, **kwargs):
        return self.fn(*args, **kwargs)

    def partial(self, config):
        kwargs = {kw: getattr(config, kw.upper()) for kw in self.keywords_to_partial}
        return functools.partial(self.fn, **kwargs)


def configurable(*keywords_to_partial):
    def outer(fn):
        return Wrapper(fn, keywords_to_partial)
    return outer

Diff: adding keywords_to_partial in __init__.

2. @pipermerriam could you elaborate what do you mean by "by just assigning a partial function to the fn object" (Reorg helper functions #146 (comment))?

[pipermerriam]

Roughly like this.

def configurable(*keywords_to_partial):
    def outer(fn):
        def partial(config):
            kwargs = {keyword: getattr(config, keyword) for keyword in keywords_to_partial}
            return functools.partial(fn, **kwargs)
        fn.partial = partial
        return fn
    return outer

No need to wrap it in a class. Just gives each function an assigned partial property that can be used to easily generate a partial'd function that pulls the values out of the configuration object. Real implementation should probably have more safety checks for things like ensuring that keywords_to_partial only contains valid values.

[ralexstokes]

closing this issue due to staleness. we can revisit if it is still relevant