bids() input lists and input zip lists

[pvandyken]

A while back, we discussed how the current attributes available on the dict returned by generate_inputs() are somewhat confusing, especially "input_lists" and "input_zip_lists". The challenge with "input_lists" is that when you use it with expand(), it will expand to all possible combinations of bids attributes, some of which may not exist. The fastest work-around if this attribute must be used is probably to filter the expand() list with Path.exists().

On the other hand, input_zip_lists will always return files that do exist, but for it to work correctly, users must call expand() as expand("/path/to/{wildcard}", zip, generate_inputs(...)["input_zip_lists"]). The zip is necessary to change the default behaviour of expand away from product.

I would propose two things to remedy this. First, add a function to the return of generate_inputs() called expand() or something similar that takes one positional argument: the path to be expanded. It will call the snakemake.io.expand() function using zip and all of the bids parameters originally passed to generate_inputs(). It will also have a **kwargs intended for any additional arguments. In this case, two expands() will be performed, the first using zip and the bids parameters, the second using product and the **kwargs. Something like:

from snakemake.io import expand

def sb_expand(path, **kwargs):
    x = expand(path, zip, **generated_inputs["input_zip_lists"]
    return expand(x, **kwargs)

Optionally, we could also allow the user to specify **kwargs that were already passed to generate_inputs() as bids parameters. These would be used to filter the bids parameters. However, this feature would be inherently messy, and my personal preference would be for users to use the filter capabilities within generate_inputs().

Second, in order to simplify the use and documentation of the generate_inputs() return, I suggest transitioning it from a Dict to a class called BidsInputs. This transition would need to be gradual to prevent breaking changes. First, add a keyword arg to generate_inputs() called use_dict that defaults to None. If False, generate_inputs() will return the BidsInputs class instead of the Dict. If True, a Dict will be used as before. If left as None, we will initially treat it as True to prevent breaking changes, but a deprecation warning will print saying that in a few versions, use_dict will default to False. (Note that if we use attrs to define the class, we can use attr.asdict to get a Dict for free, so it won't be a lot of extra logic to maintain both).

One last point as a corollary to the above suggestion: switching to a class over a Dict would prevent update()ing the config with generate_inputs(), as currently recommended by the tutorial. However, I would suggest that recommendation should be phased out anyway, as it needlessly obfuscates the generate_inputs() return value . If an unaccustomed user sees config["input_lists"] in the Snakefile, they might assume "input_lists" is a variable defined in config.yaml and become mystified when they fail to find it. config.update(generate_inputs(...)) secretly adds all those values to the config, and the only way to find out what values it adds is by reading the Snakebids documentation. In my own code, I store the generate_inputs() in a variable called inputs, and I'd suggest this as a clearer standard.

[akhanf]

I like this idea. Curious to see how BidsInput would be structured, would it be based off something that already exists?

[pvandyken]

Exactly the same as the current Dict, just a class. This would let us write docstrings for Sphinx and get auto api documentation specific for the return, rather than having it all semi-hidden under generate_inputs. It's also easier to maintain:

class BidsInput:
    input_path: str
    input_zip_lists: Dict[str, List[List[str]]]
    input_lists: Dict[str, List[str | List[str]]]
    input_wildcards: Dict[str, str]
    subjects: List[str]
    sessions: List[str]
    subj_wildcards: Dict[str, str]
    expand: Callable[[str, ...], List[str]]

[akhanf]

yep that all makes sense to me!

[tkkuehn]

I'm fully on board for transitioning the output of generate_inputs to a class.

I like the idea in principle of providing a shorthand for the long expand calls with input_zip_lists -- returning it as part of the dict seems just a little awkward, but as a method of a BidsInput class it definitely be cleaner.

[pvandyken]

Yeah, you would have a strange inputs['expand'](path) syntax. So we could put it only on the class and it can be an extra "perk" of transitioning to the new class syntax. What do you think?

[tkkuehn]

Sounds good to me!

[kaitj]

This sounds good to me - like the idea of transitioning to a class.