8000 Use additional metadata by christopher-mohr · Pull Request #59 · nf-core/epitopeprediction · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use additional metadata #59

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 3 commits into from
Sep 23, 2020
Merged

Use additional metadata #59

merged 3 commits into from
Sep 23, 2020

Conversation

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

This should add functionality to add metadata provided with variants dynamically without being limited to a set of (hardcoded) metadata fields. Which information from VCF files should we store and write to the results table? Currently, the information from the INFO column is used but we could additionally store the genotype-specific information, e.g. in the following line that would be the information given for NORMAL and TUMOR.

#CHROM	POS	ID	REF	ALT	QUAL	FILTER	INFO	FORMAT	NORMAL	TUMOR
chr1	12954870	.	C	T	.	PASS	NT=ref;QSS=52;QSS_NT=52;SGT=CC->CT;SOMATIC;TQSS=1;TQSS_NT=1;ANN=T|missense_variant|MODERATE|PRAMEF10|ENSG00000187545|transcript|ENST00000235347|protein_coding|3/4|c.413G>A|p.Cys138Tyr|493/1525|413/1425|138/474||	DP:FDP:SDP:SUBDP:AU:CU:GU:TU	414:0:0:0:0,0:413,425:0,0:1,4	8:0:0:0:0,0:3,4:0,0:5,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

@ggabernet
Copy link
Member

@christopher-mohr , you should update the changelog, but otherwise it looks good to me 👍

@ggabernet
Copy link
Member

Hi @christopher-mohr , yes we could add the extra info that you mention. Things like AF would be annotated in that field right? That might be useful for the report and some customers were asking for it

@christopher-mohr
Copy link
Collaborator Author

Hi @christopher-mohr , yes we could add the extra info that you mention. Things like AF would be annotated in that field right? That might be useful for the report and some customers were asking for it

Exactly, if AF would be available it would be added as well. I would just create columns for every "property" like TUMOR.DP, NORMAL.DP e.g. for the example above.

The question is if we should make it optional (via parameter) since this might result in a result file with many columns.

@ggabernet
Copy link
Member

However you think is reasonable would be fine I guess. To be fair as part of a table I usually do not mind if there is too much information and will use only the fields that I need. But if you think it might be confusing, then it can be controlled by a param.

skrakau
skrakau approved these changes Sep 22, 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 to me too! As Gisela said, an update to the CHANGELOG would be good though.

Regarding the metadata and the prediction report, I wouldn't think it is really necessary, unless you were thinking of a concrete use case.

@christopher-mohr
Copy link
Collaborator Author

Looks good to me too! As Gisela said, an update to the CHANGELOG would be good though.

Regarding the metadata and the prediction report, I wouldn't think it is really necessary, unless you were thinking of a concrete use case.

Do you mean a concrete use case for the parameter to enable/disable metadata or the additional metadata in general?

@skrakau
Copy link
Member
skrakau commented Sep 22, 2020

I was referring to your comment
https://github.com/christopher-mohr/nf-core-epitopeprediction/blob/c30aaac6110f8ea9a7eb22bb992f3a85270ceb89/bin/epaa.py#L225
(although this would only be written to the .command.out I realised now).

Regarding the output of the metadata to the actual results, a parameter might be useful (but it would be also fine without).

@christopher-mohr
Copy link
Collaborator Author

I was referring to your comment
https://github.com/christopher-mohr/nf-core-epitopeprediction/blob/c30aaac6110f8ea9a7eb22bb992f3a85270ceb89/bin/epaa.py#L225
(although this would only be written to the .command.out I realised now).

Regarding the output of the metadata to the actual results, a parameter might be useful (but it would be also fine without).

Ah, I see. This was basically just a placeholder for now. The data given by vcf_reader.metadata can be something like the version of the annotation software used and so on (depending on what is stored in the VCF comment lines). This would have to go to the prediction_report if we decide to store it in my opinion.

@christopher-mohr christopher-mohr merged commit 22c6edf into nf-core:dev Sep 23, 2020
@christopher-mohr christopher-mohr mentioned this pull request Sep 23, 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.
3 participants
@christopher-mohr @ggabernet @skrakau
0