Skip to content

Adjusted fixes for AWS index handling#500

Merged
apeltzer merged 56 commits intodevfrom
fix-aws
Aug 3, 2020
Merged

Adjusted fixes for AWS index handling#500
apeltzer merged 56 commits intodevfrom
fix-aws

Conversation

@apeltzer
Copy link
Copy Markdown
Member

Our handling of indices wasn't properly done I fear, so this is an attempt to get that fixed :-)

PR checklist

  • This comment contains a description of changes (with reason)
  • If you've fixed a bug or added code that should be tested, add tests!
  • If necessary, also make a PR on the nf-core/eager branch on the nf-core/test-datasets repo
  • Ensure the test suite passes (nextflow run . -profile test,docker --paired_end).
  • Make sure your code lints (nf-core lint .).
  • Documentation in docs is updated
  • CHANGELOG.md is updated
  • README.md is updated

@jfy133 jfy133 marked this pull request as draft July 15, 2020 06:32
@jfy133 jfy133 linked an issue Jul 15, 2020 that may be closed by this pull request
@jfy133 jfy133 marked this pull request as ready for review July 29, 2020 18:26
@jfy133
Copy link
Copy Markdown
Member

jfy133 commented Jul 29, 2020

@apeltzer and I got the benchmarking_vikingfish profile to work on AWS 🎉 !

However the remaining tests don't work properly (see my notes)

## TO CHECK! No mapping?
nextflow run ../../main.nf -profile test_tsv,docker --skip_collapse
## TO CHECK - one sample?
nextflow run ${GITHUB_WORKSPACE} -profile test_tsv_pretrim,docker --skip_trim
## TO CHECK - no mapping?
nextflow run ${GITHUB_WORKSPACE} -profile test_tsv,docker --skip_adapterremoval
## TO CHECK - no mapping?
nextflow run ${GITHUB_WORKSPACE} -profile test_tsv_bam,docker --skip_adapterremoval --run_convertinputbam

Copy link
Copy Markdown
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now. This includes a lot of minor fixes as well as AWS. But should be good once our final AWS pass goes (dreamy_venter). Then MERGE @apelter (finally!).

Up to you if you want to run the small test profiles too on AWS first...

Comment thread docs/usage.md
#### `--bwa_index`

If you want to use pre-existing `bwa index` indices, please supply the path **and file** to the FASTA you also specified in `--fasta` (see above). EAGER2 will automagically detect the index files by searching for the FASTA filename with the corresponding `bwa` index file suffixes.
If you want to use pre-existing `bwa index` indices, please supply the **directory** to the FASTA you also specified in `--fasta` (see above). EAGER2 will automagically detect the index files by searching for the FASTA filename with the corresponding `bwa` index file suffixes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If you want to use pre-existing `bwa index` indices, please supply the **directory** to the FASTA you also specified in `--fasta` (see above). EAGER2 will automagically detect the index files by searching for the FASTA filename with the corresponding `bwa` index file suffixes.
If you want to use pre-existing `bwa index` indices, please supply the **directory** to the FASTA you also specified in `--fasta` (see above). nf-core/eager will automagically detect the index files by searching for the FASTA filename with the corresponding `bwa` index file suffixes.

@apeltzer apeltzer merged commit 8855466 into dev Aug 3, 2020
@apeltzer apeltzer deleted the fix-aws branch August 6, 2020 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some more AWS issues

2 participants