Multiinfo

[chrisamiller]

overhauls vcf-info-annotator:

• Multi-column annotation (--column-mappings / -m)
  old: accepted a single info_field argument with separate -f/-d flags for format and description.
  new: -m/--column-mappings flag that accepts a comma-separated list of source_col:info_field:type:description mapping
  allows multiple TSV columns to be written to multiple INFO fields. Source and version metadata can optionally be appended as 5th and 6th colon-separated fields

• TSV format now requires a header row.

• does some sanity checking on fields in coerce_value

  • empty string pass through as "."
  • NA/NaN/Inf values handled appropriately
  • entries that can't be coerced will cause a crash ("red" can't be coerced to integer)

Points for discussion:

• change in how overwrite works:

  • previously, retained the original INFO header line, now replaces with the definition in arguments
  • adds a --clear-existing flag that works with --overwrite. If just --overwrite is specified, existing annotations from positions not in the new tsv file are retained. If --overwrite and --clear-existing are specified, existing annotations that don't match with positions in the new file are removed

• Reverse compatibility is broken - if we keep these changes, we'll have to update some pipelines downstream. We could shoehorn in a way to maintain the old behavior, but this felt cleaner. We should talk, though!

[chrisamiller]

This would close #46 (though in a slightly more complex way)

Also, the tests are going to continue to fail until PR #86 is merged

[susannasiebert]

I think the coerce_value method is missing, which is what is causing the test failures.

[susannasiebert]

I think some of the new methods are missing. Not sure what happened there.

[susannasiebert]

Suggested change

	def parse_tsv_file(args):
	def parse_tsv_file(args, mappings):

[susannasiebert]

I'm not seeing a parse_column_mappings method.

However, you could consider making this parsing part of the argparse argument parsing. Have a look at this example from pVACtools. You can handle the parsing (and any error checking) in a method like that and then set that method as the argument's type. That way args.column_mapping is already parsed into whatever format you'd like and the argument parsing fails upstream if an error is triggered, e.g. when the argument doesn't match the expected format.

[chrisamiller]

I somehow got my branches crossed up and this is a mess. Give me a few to untangle this - sorry!

[chrisamiller]

Okay - I've fixed the screwy merge (had to recreate some code that I inadvertently nuked) - sorry! Please take a look at this one when you get a minute to see if it's more comprehensible.

Same basic list of changes from the parent comment applies. I've also moved the validation into the argument parser, which is a nice improvement, thanks for the suggestion.

[chrisamiller]

and added explicit checking for VCF valid types: ['Integer', 'Float', 'Flag', 'Character', 'String']

[susannasiebert]

+1