Add genetic map support for all tools #216
Conversation
atrigila
left a comment
atrigila
left a comment
There was a problem hiding this comment.
Since this is a pretty extensive change, I would benefit from reviewing it in incremental stages. I suggest you make PR's to modules. There, you can submit the local subworkflows and add the tests that require using maps. Once we get those approved, it will be easier to then focus on the broader picture here with tests aiming at the whole pipeline tests.
| conda "${moduleDir}/environment.yml" | ||
| container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ? | ||
| 'https://depot.galaxyproject.org/singularity/gawk:5.3.0' : | ||
| 'biocontainers/gawk:5.3.0' }" |
There was a problem hiding this comment.
If we are using GAWK, why don't we use the nf-core module?
There was a problem hiding this comment.
The issue to use the nf-core module is that it would result in a more complex looking module (script in the config), splitting output channel based on regex and therefore a more complicated modules to maintain.
Writing it this may does add a new local module, but make it easier to understand how it works.
docs/usage.md
Outdated
| `--map_sep "\t" --map_header true --map_col_names "chr,id,cm,pos"` | ||
|
|
||
| ```csv title="chr21.map" | ||
| chr id centimorgan position |
There was a problem hiding this comment.
Your header here is centimorgan position while in the description you say that it should be pos and cm
There was a problem hiding this comment.
The header is in fact not parsed but can be present, so the columns names in the map file are not useful.
| - `--map_sep`: Field separator used in the map file (e.g. "\t", " ", ",") | ||
| - `--map_header`: Whether the file contains a header row (`true` or `false`) | ||
| - `--map_col_names`: Ordered list of column names in the file |
There was a problem hiding this comment.
I think these could be easily inferred in the module so that we don't add new params to users (which means they might be more prone to errors).
There was a problem hiding this comment.
Shouldn't we have a test for all the other imputation tools that require a map? A test with and without a map.
2 participants
PR checklist
This PR add genetic map support for phasing and imputation tool as well as auto convertion to all format.
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).