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.

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

VariantRDD union creates multiple records for the same SNP ID #1644

Closed
devin-petersohn opened this Issue Jul 27, 2017 · 6 comments

Comments

Projects
3 participants
@devin-petersohn
Member

devin-petersohn commented Jul 27, 2017

VariantRDD.union() does not combine by SNP ID. This leads to the potential for multiple records with the same SNP ID.

If we were to write out the VariantRDD as a VCF, it would not be correct.

@devin-petersohn

This comment has been minimized.

Show comment
Hide comment
@devin-petersohn
Member

devin-petersohn commented Jul 27, 2017

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Jul 27, 2017

Member

We already split alternate alleles at the same position into multiple Variant records, so I would not expect VariantRDD.union to combine by Variant.name. I would in fact expect to see many records at the same position with the same value(s) for Variant.name.

  /**
   Zero or more unique names or identifiers for this variant. If this is a dbSNP
   variant it is encouraged to use the rs number(s). VCF column 3 "ID" shared across
   all alleles in the same VCF record.
   */
  array<string> names = [];
Member

heuermh commented Jul 27, 2017

We already split alternate alleles at the same position into multiple Variant records, so I would not expect VariantRDD.union to combine by Variant.name. I would in fact expect to see many records at the same position with the same value(s) for Variant.name.

  /**
   Zero or more unique names or identifiers for this variant. If this is a dbSNP
   variant it is encouraged to use the rs number(s). VCF column 3 "ID" shared across
   all alleles in the same VCF record.
   */
  array<string> names = [];
@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Jul 27, 2017

Member

+1 @heuermh, the union method specifies no contract WRT "duplicate" records.

Logically, in variant space, "duplicate" is not clearly defined, as two records with the same chr/pos/ref/alt could have conflicting annotations. I don't believe that the VCF spec says that dupe records are illegal, as long as both records are well formed and the data is properly sorted.

I suggest we close as won't fix.

Member

fnothaft commented Jul 27, 2017

+1 @heuermh, the union method specifies no contract WRT "duplicate" records.

Logically, in variant space, "duplicate" is not clearly defined, as two records with the same chr/pos/ref/alt could have conflicting annotations. I don't believe that the VCF spec says that dupe records are illegal, as long as both records are well formed and the data is properly sorted.

I suggest we close as won't fix.

@devin-petersohn

This comment has been minimized.

Show comment
Hide comment
@devin-petersohn

devin-petersohn Jul 27, 2017

Member

I was thinking along the lines of the VCFTools implementation here, such that the output is a merged VCF.

If we don't care about this use case, feel free to close the issue.

Member

devin-petersohn commented Jul 27, 2017

I was thinking along the lines of the VCFTools implementation here, such that the output is a merged VCF.

If we don't care about this use case, feel free to close the issue.

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Jul 27, 2017

Member

union on GenotypeRDD/VariantRDD/VariantContextRDD implements both VCFTools merge and concat, albiet with possibly different semantics in the merge case.

Member

fnothaft commented Jul 27, 2017

union on GenotypeRDD/VariantRDD/VariantContextRDD implements both VCFTools merge and concat, albiet with possibly different semantics in the merge case.

@devin-petersohn

This comment has been minimized.

Show comment
Hide comment
@devin-petersohn

devin-petersohn Jul 30, 2017

Member

Closing as won't fix.

Member

devin-petersohn commented Jul 30, 2017

Closing as won't fix.

@heuermh heuermh added this to the 0.23.0 milestone Dec 7, 2017

@heuermh heuermh added this to Completed in Release 0.23.0 Jan 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment