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

Switch to nf-validation #220

Merged
merged 14 commits into from
Dec 19, 2023
Merged

Switch to nf-validation #220

merged 14 commits into from
Dec 19, 2023

Conversation

jonasscheid
Copy link
Contributor

@jonasscheid jonasscheid commented Dec 5, 2023 •
edited
Loading

Intitially wanted to fix an issue in the samplesheet checker with .vcf.gz, but then I realized we should move to nf-validation to get rid of some custom code and properly check parameters.

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 you've added a new tool - have you followed the pipeline conventions in the contribution docs- [ ] If necessary, also make a PR on the nf-core/epitopeprediction branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 5, 2023 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 693bd50

+| ✅ 160 tests passed       |+
!| ❗   1 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in WorkflowEpitopeprediction.groovy: Optionally add in-text citation tools to this list.

✅ Tests passed:

Run details

  • nf-core/tools version 2.10
  • Run at 2023-12-19 11:28:34

@jonasscheid
Copy link
Contributor Author

@christopher-mohr Do you know if if anyone still uses the variant tsv format or GSvar? Can we deprecate it? Does not seem to be a community-format 🤔 or am I mistaken

@christopher-mohr
Copy link
Collaborator

community-format

How do you define community-format? In first place this was implemented since we usually received the data in this format. Not sure what is used at the moment at QBiC etc. However, in general this should not be part of this PR.

@jonasscheid
Copy link
Contributor Author

How do you define community-format?

A standardised text file format that represents information about SNPs, indels and structural variants that the majority of scientists use.

Not sure what is used at the moment at QBiC etc.

Not used as people told me

However, in general this should not be part of this PR.

True, but since it is related to the errors, I was questioning whether these formats will be maintained and whether we should support them. Otherwise, I would set up another PR deprecating the related tests and functionality

@christopher-mohr
Copy link
Collaborator

True, but since it is related to the errors, I was questioning whether these formats will be maintained and whether we should support them. Otherwise, I would set up another PR deprecating the related tests and functionality

The problem seems to be that you are providing a tsv file with variants as peptide input.

@jonasscheid
Copy link
Contributor Author

That is why I asked regarding maintaining these variant tsv is still a thing we want to do.

Otherwise, I would set up another PR deprecating the related tests and functionality

of variant tsv and GSvar

@christopher-mohr
Copy link
Collaborator

That is why I asked regarding maintaining these variant tsv is still a thing we want to do.

Otherwise, I would set up another PR deprecating the related tests and functionality

of variant tsv and GSvar

Yes I know. :) We should just be sure that it's not a problem in general with the added changes.

@jonasscheid
Copy link
Contributor Author

Ok, so then I'll set up a PR depricating tsv and GSvar (btw. IMGAG preferring vcf over GSvar) and we make sure that the added changes here don't introduce any general problems 👍🏼

This was referenced Dec 8, 2023
Merged
Merged
christopher-mohr
christopher-mohr reviewed Dec 19, 2023
View reviewed changes
Copy link
Collaborator

@christopher-mohr christopher-mohr left a comment

Choose a reason for hiding this comment

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

Just some minor things

workflows/epitopeprediction.nf Outdated Show resolved Hide resolved
workflows/epitopeprediction.nf Outdated Show resolved Hide resolved
workflows/epitopeprediction.nf Show resolved Hide resolved
christopher-mohr
christopher-mohr approved these changes Dec 19, 2023
View reviewed changes
Copy link
Collaborator

@christopher-mohr christopher-mohr left a comment

Choose a reason for hiding this comment

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

LGTM

@jonasscheid jonasscheid merged commit ce8c16d into nf-core:dev Dec 19, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christopher-mohr christopher-mohr christopher-mohr 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.
2 participants
@jonasscheid @christopher-mohr