Complete refactoring of variant and related annotation records

[heuermh]

Fixes #96.

Still needs work; additional changes to come based on VCF mapping review.

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/bdg-formats-prb/116/
Test PASSed.

[fnothaft]

s/variable/variant/g? Or, is variable meant to indicate that the copy number of this region varies?

[heuermh]

verbatim from spec, it sucks but ... ?

[fnothaft]

Actually, it's not too bad: the crux is "this region has varied copy number across samples". A "copy number variant" would be the specific change in copy number. I actually think this is the right call.

[heuermh]

I agree, though unfortunate TLA collision.

[fnothaft]

Yeah, no joke.

[fnothaft]

LGTM other than two questions.

[jpdna]

LGTM other than two questions.

What are the remaining two questions or other issues on this PR?

[fnothaft]

Thanks for pinging @jpdna. They were the two inline comments I added, but @heuermh answered the questions and I didn't think we needed any additional changes, so this is +1 from my end.

[jpdna]

Does the complementary code for the ADAM repo to populate the array<string> names in Variant exist now in any PR, or is that a task we need to to open an issue for?
Even if that ADAM repo code is not ready yet - are ready to go ahead and merge this bdg-formats PR now in prep for that?

[fnothaft]

Does the complementary code for the ADAM repo to populate the array names in Variant exist now in any PR, or is that a task we need to to open an issue for?

I don't think the code is done yet. I think it might be added to bigdatagenomics/adam#1078, but I'd wait for @heuermh to comment.

Even if that ADAM repo code is not ready yet - are ready to go ahead and merge this bdg-formats PR now in prep for that?

+1

[jpdna]

So - what's the SOP on merging bdg-formats PRs? I see we don't have a scripts/commit-pr.sh as we do for ADAM.

[fnothaft]

Yeah, we just hit the green merge button here. It's pretty rare that we:

1. have multiple active PRs in this repo
2. have PRs that need huge complex rebases

in this repo, so the ./scripts/commit-pr.sh script isn't as necessary for bdg-formats.

[jpdna]

Thanks @heuermh
merged

[heuermh]

No! This wasn't ready to go. Sorry for not replying sooner, have been in the mountains away from cell phone service.

[jpdna]

Ah sorry - what steps should we take to roll it back? I see a handy "revert" button above in github that create a PR to revert.
Should I push the revert button?

[fnothaft]

No rush on this. We can either revert the merge, or force push HEAD prior to the merge to master. I'm fine either way. The first one is arguably the more kosher one to do.

[heuermh]

Being that there may be some concern in my ability to land all the relevant code changes before the 0.20 release of adam, please let me take the opportunity to consider rolling this and commit 5e81a9c back, and re-proposing some of the changes piecemeal.

We hadn't finished going though all the potential variant and genotype schema changes before I went on vacation, and this pull request didn't even reflect the latest conversation we had (for example, I believe we decided to remove StructuralVariant entirely).

[fnothaft]

@heuermh you're in the office next week, right? If so, let's chat through this on standup.

[heuermh]

Back by Wed for sure.

[fnothaft]

Sounds good; we'll talk then, and I hope you're having a great trip!