search

BIMSBbioinfo / pigx_sars-cov-2

PiGx SARS-CoV-2 wastewater sequencing pipeline
GNU General Public License v3.0
18 stars 3 forks source link

clean up variant_report #114

Closed alexg9010 closed 2 years ago

alexg9010 commented 2 years ago

shouldn't we filter all_variants instead of having a one-branch if here?

_Originally posted by @rekado in https://github.com/BIMSBbioinfo/pigx_sars-cov-2/pull/100#discussion_r844967748_

alexg9010 commented 2 years ago

Please invert the if condition. When using if / else always aim to use a positive condition.

_Originally posted by @rekado in https://github.com/BIMSBbioinfo/pigx_sars-cov-2/pull/100#discussion_r844966801_

alexg9010 commented 2 years ago

weird indentation.

_Originally posted by @rekado in https://github.com/BIMSBbioinfo/pigx_sars-cov-2/pull/100#discussion_r844967342_

alexg9010 commented 2 years ago

shouldn't this condition be merged with the above? This would reduce unnecessary nesting.

_Originally posted by @rekado in https://github.com/BIMSBbioinfo/pigx_sars-cov-2/pull/100#discussion_r844968194_

alexg9010 commented 2 years ago

Let's delete this instead of commenting it. It's either needed or not. Let's not clutter this already cluttered file :)

_Originally posted by @rekado in https://github.com/BIMSBbioinfo/pigx_sars-cov-2/pull/100#discussion_r844968818_

alexg9010 commented 2 years ago

Don't use hardcoded folder names. Instead, pass the full path via the report parameters and the associated snakemake rule.

https://github.com/BIMSBbioinfo/pigx_sars-cov-2/blob/4150b98d9f8be2866c5f9d56881dfdabcad1aa7c/scripts/report_scripts/variantreport_p_sample.Rmd#L50-L51

jonasfreimuth commented 2 years ago

Should all be fixed in #121

jonasfreimuth commented 2 years ago

Most of these are not even relevant any more, the rest is fixed. Probably all already in #131.