-
Notifications
You must be signed in to change notification settings - Fork 705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dev -> Master for 3.12.0 release #1040
Conversation
Bump pipeline version to 3.12.0dev
Important! Template update for nf-core/tools v2.8
The `docker.registry` configuration should always be set, as running on cloud executors will need to pull docker images but will not necessarily use the `docker` profile.
restore process.shell declaration
Corrected spelling of name "Edmund Miller".
Set a default docker registry outside of profile scope.
Update README.md
Add public_aws_ecr.config and use in CI tests
Co-authored-by: Harshil Patel <[email protected]>
Fix for public_aws_ecr profile that didn't match modules sometimes
PROFILES: test -> test_cache + restore test
…onal_fasta Update warning about additional_fasta and gene BED
Bump pipeline version to 3.12.0
|
@@ -235,7 +235,7 @@ workflow PREPARE_GENOME { | |||
ch_hisat2_index = Channel.value(file(hisat2_index)) | |||
} | |||
} else { | |||
ch_hisat2_index = HISAT2_BUILD ( ch_fasta, ch_gtf, ch_splicesites ).index | |||
ch_hisat2_index = HISAT2_BUILD ( ch_fasta.map { [ [:], it ] }, ch_gtf.map { [ [:], it ] }, ch_splicesites.map { [ [:], it ] } ).index.map { it[1] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is kinda ugly. We could add a method add_empty_map
, or even an operator to do this? Probably not worth it for this release but it would make it cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. Had to introduce it because the upstream modules been update to use meta for the references.
input = "${params.test_data_base}/samplesheet/v3.10/samplesheet_test.csv" | ||
|
||
// Genome references | ||
fasta = "${params.test_data_base}/reference/genome.fasta" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it on purpose that you replaced the ${params.test_data_base}
with the absolute Github URL in the test.config
, but kept it around here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, the test_cache.config
will be used in Github Actions so we can clone the branch with the test data (and be able to do the same in offline environments) to run the minimal tests - this is mostly because we had CI tests failing quite frequently when using absolute paths. The test.config
was reverted to be the same as other pipelines with absolute paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to do the same with methylseq and sarek
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the Github's browser-based review mode, this file to me is shown empty. But when I check out the current Dev branch in VS Code, it is there, so I suppose everything is alright.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's because only the permissions were changed on the file and not any contents.
FIX: remove mem assignement for SAMTOOLS_SORT
No description provided.