ENSGENOMIO-18 Chunking modules#34
Conversation
markquintontulloch
requested review from
Dishalodha,
JAlvarezJarreta,
shradhaebi and
vsitnik
Dishalodha
left a comment
Dishalodha
left a comment
There was a problem hiding this comment.
Partial review, will test the code when cluster is back.
Looks good overall, just minor updates.
| - conda-forge | ||
| - bioconda | ||
| dependencies: | ||
| - ensembl-genomio=1.6.1 No newline at end of file |
There was a problem hiding this comment.
I think we are in version
| - ensembl-genomio=1.6.1 | |
| - ensembl-genomio=1.6.2 |
There was a problem hiding this comment.
Will update container version once python PR is approved/merged and we can generate new package of GenomIO
| label 'process_medium' | ||
|
|
||
| conda "${moduleDir}/environment.yml" | ||
| container "ensemblorg/ensembl-genomio:v1.6.1" |
There was a problem hiding this comment.
| container "ensemblorg/ensembl-genomio:v1.6.1" | |
| container "ensemblorg/ensembl-genomio:v1.6.2-docker" |
|
|
||
| singularity { | ||
| enabled = true | ||
| enabled = false |
There was a problem hiding this comment.
why are we not enabling singularity?
There was a problem hiding this comment.
Hangover from local testing, reverted
There was a problem hiding this comment.
Do we need this folder?
There was a problem hiding this comment.
No, I'd initially implemented non-stub tests before we made the decision to only have stub tests - this was a hanger from that. Removed now.
| tag "modules" | ||
| tag "modules_ensembl" |
There was a problem hiding this comment.
I dont think these tags are required
| tag "modules" | |
| tag "modules_ensembl" |
There was a problem hiding this comment.
Actually, reinstating as required by nf-core linting
| label 'process_medium' | ||
|
|
||
| conda "${moduleDir}/environment.yml" | ||
| container "ensemblorg/ensembl-genomio:v1.6.1" |
There was a problem hiding this comment.
| container "ensemblorg/ensembl-genomio:v1.6.1" | |
| container "ensemblorg/ensembl-genomio:v1.6.2-docker" |
There was a problem hiding this comment.
I might be wrong but do we need to add this to genomio?
There was a problem hiding this comment.
Hangover from non-stub tests - removed
| - conda-forge | ||
| - bioconda | ||
| dependencies: | ||
| - ensembl-genomio=1.6.1 |
There was a problem hiding this comment.
| - ensembl-genomio=1.6.1 | |
| - ensembl-genomio=1.6.2 |
| } | ||
|
|
||
| def out_fasta = "${meta.id}.fa" | ||
|
|
| container "${workflow.containerEngine in ['singularity', 'apptainer'] && !task.ext.singularity_pull_docker_container | ||
| ? 'https://depot.galaxyproject.org/singularity/ensembl-genomio:1.6.1--pyhdfd78af_0' | ||
| : 'biocontainers/ensembl-genomio:1.6.1--pyhdfd78af_0'}" |
There was a problem hiding this comment.
| container "${workflow.containerEngine in ['singularity', 'apptainer'] && !task.ext.singularity_pull_docker_container | |
| ? 'https://depot.galaxyproject.org/singularity/ensembl-genomio:1.6.1--pyhdfd78af_0' | |
| : 'biocontainers/ensembl-genomio:1.6.1--pyhdfd78af_0'}" | |
| container "ensemblorg/ensembl-genomio:v1.6.2-docker" |
| - conda-forge | ||
| - bioconda | ||
| dependencies: | ||
| - ensembl-genomio=1.6.1 |
There was a problem hiding this comment.
| - ensembl-genomio=1.6.1 | |
| - ensembl-genomio=1.6.2 |
| container "${workflow.containerEngine in ['singularity', 'apptainer'] && !task.ext.singularity_pull_docker_container | ||
| ? 'https://depot.galaxyproject.org/singularity/ensembl-genomio:1.6.1--pyhdfd78af_0' | ||
| : 'biocontainers/ensembl-genomio:1.6.1--pyhdfd78af_0'}" |
There was a problem hiding this comment.
update container to version 2
|
|
||
| output: | ||
| tuple val(meta), path("${meta.id}.fa"), emit: recombined_fasta | ||
| tuple val("${task.process}"), val('fasta_recombine'), eval('echo 1.6.1'), emit: versions_fasta_recombine, topic: versions |
There was a problem hiding this comment.
maybe eval('echo 1.6.1') shouldn't be hard coded here, we can capture genomio version or the module version.
There was a problem hiding this comment.
Updated to use genomio version
| output: | ||
| tuple val(meta), path("splits/**/*.fa"), emit: fastas | ||
| tuple val(meta), path("splits/*.agp"), emit: agp, optional: true | ||
| tuple val("${task.process}"), val('fasta_split'), eval('echo 1.6.1'), emit: versions_fasta_split, topic: versions |
There was a problem hiding this comment.
Updated to use genomio version
| tuple val("${task.process}"), | ||
| val('fasta_split'), | ||
| eval(params.ensembl_genomio_version_cmd), | ||
| eval("python -c 'from importlib.metadata import distributions; print(next((dist.version for dist in distributions() if dist.metadata[\"Name\"].lower().replace(\"_\", \"-\") == \"ensembl-genomio\"), \"unknown\"))'"), |
There was a problem hiding this comment.
Will it be possible to use fasta_split --version directly since I guess container should pick it up
There was a problem hiding this comment.
Good point. I hadn't implemented a version parameter on fasta_split or the other python packaged. I have now added it to the current ensembl-genomio PR, so will update this to use that once that PR has been approved/merged.
There was a problem hiding this comment.
In fact, updated now since genomio-repeatmasker is using PR branch of ensembl-genomio for CICD and this branch can't be merged until containers updated. Same change made for features_combine_json.
2 participants
Creating draft pull request as following changes required:
Container version for Genomio needs to be updated once python code approved, merged, and new release container created.
Tests need to be updated in accordance with group's new policy.Tests now updated to avoid use of stored files