feat: enable user to configure CLI params per rule

Closes #155 (closed)

• added rule_config.yaml to tests/input_files/. Here, additional parameters for all rules in main Snakefile and single/paired-end Snakefiles can be set
• before each rule a variable current_rule is set to the name of the current rule. This workaround is necessary, as currently the snakemake directive {rule} can only be accessed from within the shell calls, whereas we need to access it from the params section
• Parsing the rule_config.yaml:
  • In main Snakefile, rule_config.yaml will be loaded into a rule_config dict; if no rule_config.yaml specified, rule_config dict will be empty and all tools will run with default parameters.
  • In main Snakefile, a function parse_rule_config is defined, which takes as input the rule_config dict, the current rule name, and a tuple of arguments that must not be changed for the current rule from within the rule config ("immutable" arguments) in order to ensure proper functioning of the pipeline. Immutable arguments are those that either influence a tool's behavior in a way that is critical for pipeline wiring, or those that are defined in the samples table. Returns a string with all additional parameter-value pairs.
  • in params section of each rule the above described function is called. Parameters read from the rule_config.yaml are then injected as a string in the respective shell calls to the tools
• Adapted config.yaml for tests accordingly
• Adapted pipeline documentation accordingly
• The variable current_rule is used for logfile naming in each rule
• Adapted zarp_multiqc_config.py accordingly

This looks great! One question though (before looking at the code in more detail): Can you elaborate why exactly tests fail? Is it because the tests need specific values for certain tool params in order to produce exactly the outputs that we expect (and provide and verify the MD5 sums of)?

Sorry for the vague description. Yes, it is because "the tests need specific values for certain tool params in order to produce exactly the outputs that we expect (and provide and verify the MD5 sums of)". I guess I needn't have mentioned the failing of tests in case of a missing rule_config, because obviously when using a different sample.tsv file the tests will also fail...Sorry

I remember now why I mentioned the possible failing of tests at all: The samples.tsv resides in tests/input_files, whereas the rule_config.yaml is in config/. Consequently it might not be obvious why tests might fail. Should we move the rule_config.yaml to tests/input_files? Not sure about the most consistent way here... How will ZARP-cli place these files?

Yes, rule_config.yaml should go in tests/input_files and its location should be set in tests/input_files/config.yaml, as it is clearly an input for a specific test. In ZARP-cli, I'm imagining that there will be a CLI option to supply the rule config file. If not provided, a default file will be used (which is going to be located in ~/.zarp/rule_config.yaml). During ZARP-cli initialization, the user will be prompted to create such a defaults file. If the user chooses not to create the file, rules will be run with default params.

As for providing a template/example for this, I think we can point to the rule config (or configs, in case we will create more tests) in tests/input_files.

Do you see any issues with this approach, @herrmchr?

added 1 commit

• 019a1037 - rule_config moved to tests/input_files/

Compare with previous version

changed the description

Missing space in accuracy,higher

Actually, there is also a typo in line 107: Consider using lower values of poor mapping is observed. Should be if poor mapping is observed. Might as well change that here

changed this line in version 3 of the diff

I assume we cannot use the value of current_rule here instead of repeating the string (which is error-prone), right?

couldn't figure out how. Just writing rule current_rule gives an error as would be expected. Couldn't find a way to access the variable value in the rule directive. Any ideas? Snakemake doc mentions anonymous rules but I don't quite understand the implications

It's fine, just leave it as it is

Might be clearer/more legible to have these lines wrapped (and more in accordance with PEP-8 line length limitations), e.g.:

additional_params = parse_rule_config(
    rule_config,
    current_rule=current_rule,
    immutable=('-a', '-A', '-g', '-G', '-o', '-p'),
)

or even:

additional_params = parse_rule_config(
    rule_config,
    current_rule=current_rule,
    immutable=(
        '-a',
        '-A',
        '-g',
        '-G',
        '-o',
        '-p',
    ),
)

changed this line in version 3 of the diff

added 1 commit

• 1f97d055 - typos, formatting

Compare with previous version

approved this merge request

changed title from Rule config params to feat: enable user to configure CLI params per rule

merged

mentioned in commit 10e8891c