Skip to content
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.

Sign up for GitHub

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

Jump to bottom

FIX: Remove use_annotation_cache_keys params #1298

Merged
merged 14 commits into from
Nov 1, 2023
Merged

FIX: Remove use_annotation_cache_keys params #1298

merged 14 commits into from
Nov 1, 2023

Conversation

maxulysse
Copy link
Member

No description provided.

@github-actions
Copy link

github-actions bot commented Oct 27, 2023 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 290f018

+| ✅ 144 tests passed       |+
#| ❔   9 tests were ignored |#
!| ❗   4 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in WorkflowSarek.groovy: Optionally add in-text citation tools to this list.
  • schema_description - No description provided in schema for parameter: bcftools_annotations
  • schema_description - No description provided in schema for parameter: bcftools_annotations_index
  • schema_description - No description provided in schema for parameter: bcftools_header_lines

❔ Tests ignored:

  • files_exist - File is ignored: .github/workflows/awsfulltest.yml
  • files_exist - File is ignored: conf/modules.config
  • files_unchanged - File ignored due to lint config: assets/nf-core-sarek_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-sarek_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-sarek_logo_dark.png
  • files_unchanged - File ignored due to lint config: lib/NfcoreTemplate.groovy
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore or pyproject.toml
  • actions_ci - actions_ci
  • template_strings - template_strings

✅ Tests passed:

Run details

  • nf-core/tools version 2.10
  • Run at 2023-11-01 10:34:36

asp8200
asp8200 requested changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@asp8200 asp8200 left a comment

Choose a reason for hiding this comment

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

Unfortunately, still doesn't work for me, see posts on Slack:

https://nfcore.slack.com/archives/C05V9FRJYMV/p1698424331245949?thread_ts=1698416130.722429&cid=C05V9FRJYMV

@asp8200
Copy link
Contributor

asp8200 commented Oct 30, 2023

The following cmd fails because of the hardcoding of the Sarek version number to 3.3.0

https://github.com/maxulysse/nf-core_sarek/blob/d65ecf662ef815aa5c9b31d6212637703c33be62/docs/usage.md?plain=1#L991

I get Please specify either --download_cacheor--snpeff_cache, --vep_cache.

However, I get the same error when running the command from the docs on your latest commit:

nextflow run main.nf  --outdir results --outdir_cache /home/ubuntu/test_cache --tools vep,snpeff --download_cache --build_only_index --input false

The command in the docs should probably also include --vep_cache=false and --snpeff_cache=false.

maxulysse
maxulysse commented Oct 31, 2023
View reviewed changes
subworkflows/local/samplesheet_to_channel/main.nf
Comment on lines -257 to -259
if ((params.download_cache) && (params.snpeff_cache || params.vep_cache)) {
error("Please specify either `--download_cache` or `--snpeff_cache`, `--vep_cache`.\nhttps://nf-co.re/sarek/usage#how-to-customise-snpeff-and-vep-annotation")
}
Copy link
Member Author

Choose a reason for hiding this comment

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

removed that bit

@asp8200
Copy link
Contributor

asp8200 commented Oct 31, 2023

Inusage.md, there are commands like

nextflow run nf-core/sarek -r 3.3.0 --outdir results --outdir_cache /path_to/my-own-cache --tools vep,snpeff --download_cache --build_only_index --input false

where the version of Sarek is hardcoded to 3.3.0. Could one instead specify -r master? If not, then I suggest replacing r 3.3.0 with the version number of the next release of Sarek.

There is another command there with -r 3.3.0 which probably also needs to be updated.

By the way, I needed to add -profile docker to actually run the commands.

@maxulysse
Copy link
Member Author

Inusage.md, there are commands like

nextflow run nf-core/sarek -r 3.3.0 --outdir results --outdir_cache /path_to/my-own-cache --tools vep,snpeff --download_cache --build_only_index --input false

where the version of Sarek is hardcoded to 3.3.0. Could one instead specify -r master? If not, then I suggest replacing r 3.3.0 with the version number of the next release of Sarek.

There is another command there with -r 3.3.0 which probably also needs to be updated.

By the way, I needed to add -profile docker to actually run the commands.

Good point about the version, I'll update that.

For the profile, I'd say it's implied given the previous instructions that you have to use one

FriederikeHanssen
FriederikeHanssen reviewed Nov 1, 2023
View reviewed changes
Copy link
Contributor

@FriederikeHanssen FriederikeHanssen left a comment

Choose a reason for hiding this comment

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

looks good. but why remove -profile?

docs/usage.md
@@ -962,7 +962,7 @@ aws s3 --no-sign-request ls s3://annotation-cache/vep_cache/

Since both Snpeff and VEP are internally figuring the path towards the specific cache version / species, `annotation-cache` is using an extra set of keys to specify the species and genome build.

So if you are using this resource, please either set `--use_annotation_cache_keys` to use the AWS annotation cache, or point towards your own cache folder structure matching the expected structure.
Which is handled internally by Sarek.
Copy link
Contributor

Choose a reason for hiding this comment

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

music to my user-ears. But what is Whichpointing to?

Copy link
Member Author

Choose a reason for hiding this comment

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

annotation-cache is using an extra set of keys to specify the species and genome build.

docs/usage.md Show resolved Hide resolved
@maxulysse
Copy link
Member Author

looks good. but why remove -profile?

I felt it was a mess because we have that in some command but not others, so I'd rather remove it everywhere and tell people to use the profile they need

@maxulysse maxulysse merged commit 878cdbb into nf-core:dev Nov 1, 2023
25 checks passed
@maxulysse maxulysse deleted the annotation_cache branch November 1, 2023 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asp8200 asp8200 asp8200 requested changes

@FriederikeHanssen FriederikeHanssen FriederikeHanssen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@maxulysse @asp8200 @FriederikeHanssen