8000 Add functionality to write fasta with protein sequences by christopher-mohr · Pull Request #57 · nf-core/epitopeprediction · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add functionality to write fasta with protein sequences #57

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
Merged
merged 5 commits into from
Sep 18, 2020
Merged

Add functionality to write fasta with protein sequences #57

merged 5 commits into from
Sep 18, 2020

Conversation

christopher-mohr
Copy link
Collaborator
@christopher-mohr christopher-mohr commented Sep 11, 2020 •
edited
Loading

Add functionality to write out a FASTA file with protein sequences as mentioned in #9

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/epitopeprediction branch on the nf-core/test-datasets repo
  • Ensure the test suite passes (nextflow run . -profile test,docker).
  • Make sure your code lints (nf-core lint .).
  • Documentation in docs is updated
  • CHANGELOG.md is updated
  • README.md is updated

Learn more about contributing: https://github.com/nf-core/epitopeprediction/tree/master/.github/CONTRIBUTING.md

skrakau
skrakau requested changes Sep 14, 2020
View reviewed changes
Copy link
Member
@skrakau skrakau left a comment

Choose a reason for hiding this comment

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

Hi, great that you added this! It is addressing #9 (not #33, although both dealing with FASTAs). I added a few comments.

Could you additionally add a description to the docs/output.md?

And if I see it right, this option only works (and makes sense) with variants as input, i.e. not with --peptides or --proteins. Maybe it would be useful to check at the beginning of the pipline if the parameter combination is valid and throw an error if not. Also this could be mentioned in the help/schema description of this parameter.

@christopher-mohr
Copy link
Collaborator Author

Hi, great that you added this! It is addressing #9 (not #33, although both dealing with FASTAs). I added a few comments.

Right, changed it! 👍

@christopher-mohr
Copy link
Collaborator Author

Thanks for the review @skrakau! Hope you're happy with the changes :)

skrakau
skrakau requested changes Sep 15, 2020
View reviewed changes
Copy link
Member
@skrakau skrakau 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, just one tiny detail is still missing ;)

@christopher-mohr
Copy link
Collaborator Author

The last commit includes more changes than initially planned ;):

  • merge FASTA files in case of splitted results
  • fixed the statistics for single and multiple (splitter) results
  • include input base name in result files
  • change result folder name
  • update docs

skrakau
skrakau requested changes Sep 17, 2020
View reviewed changes
Copy link
Member
@skrakau skrakau 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! :)

Just one remaining inconsistency in the notation of output files.

skrakau
skrakau approved these changes Sep 18, 2020
View reviewed changes
Copy link
Member
@skrakau skrakau left a comment

Choose a reason for hiding this comment

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

LGTM @christopher-mohr

@christopher-mohr
Copy link
Collaborator Author

LGTM @christopher-mohr

Thank you @skrakau !

@christopher-mohr christopher-mohr merged commit 70dac19 into nf-core:dev Sep 18, 2020
@christopher-mohr christopher-mohr mentioned this pull request Sep 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skrakau skrakau skrakau 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
@christopher-mohr @skrakau
0