Getter for generic fields in VCFSimpleHeaderLine

[Wathis]

Description

Setting a getter for accessing to values like "Description"

Checklist

• [] Code compiles correctly
• [] New tests covering changes and new functionality
• [] All tests passing
• [] Extended the README / documentation, if necessary
• [] Is not backward compatible (breaks binary or source compatibility)

[lbergelson]

@Wathis Thanks for contributing to htsjdk! This is a good idea. It's strange that we don't have any mechanism for getting optional fields here. However, your PR doesn't compile because you're returning String instead of Map<String, String>.

I've suggested a way of resolving the compilation failure. It would be great if you could add a test to VCFHeaderUnitTest that just asserts that you get back the values you expect. We like to have coverage of all new code even if it's something simple like this. (The good news is that simple code is usually simple to test...)

@cmnbroad Do you have any thoughts about this? It's weird that there aren't methods to get these. It seems like there isn't even a way to represent optional key/pairs in VCFCompoundHeaderLine even though it's definitely allowed in the spec

[lbergelson]

Suggested change

	public String getGenericFields() {
	/**
	* @return a map of all pairs of fields and values in this header line
	*/
	public Map<String, String> getGenericFields() {
	return Collections.unmodifiableMap(genericFields);
	}

[Wathis]

@lbergelson Yes and sorry for String ;). For unit test, no problem i will do it when i have time!

[cmnbroad]

@lbergelson I think its fine sense given the current implementation. Ideally, generic field support would live higher in the class hierarchy since all (ID) header lines can have custom attributes. The existing hierarchy only implements them for VCFSimpleHeaderLine derivatives (contig and filter), which is strange. (This is another area that was unified and fixed in my old PR that never got merged).

[Wathis]

I added some test and changed the getter too ! Now it's working for me

[codecov-io]

Codecov Report

Merging #1212 into master will increase coverage by 0.18%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##              master     #1212      +/-   ##
==============================================
+ Coverage     68.702%   68.882%   +0.18%     
- Complexity      8061      8073      +12     
==============================================
  Files            542       540       -2     
  Lines          32711     32685      -26     
  Branches        5533      5527       -6     
==============================================
+ Hits           22473     22514      +41     
+ Misses          8038      7972      -66     
+ Partials        2200      2199       -1

Impacted Files	Coverage Δ	Complexity Δ
...n/java/htsjdk/variant/vcf/VCFSimpleHeaderLine.java	79.487% <ø> (+0.54%)	12 <0> (+1)	⬆️
...samtools/util/AsyncBlockCompressedInputStream.java	72% <0%> (-4%)	12% <0%> (-1%)
...va/htsjdk/samtools/sra/SRAIndexedSequenceFile.java	62.162% <0%> (-2.703%)	6% <0%> (-1%)
src/main/java/htsjdk/samtools/cram/io/ITF8.java	94.505% <0%> (-2.046%)	23% <0%> (+1%)
src/main/java/htsjdk/samtools/cram/io/LTF8.java	96.032% <0%> (-1.509%)	19% <0%> (+1%)
src/main/java/htsjdk/samtools/SAMFileHeader.java	67.442% <0%> (-0.822%)	45% <0%> (ø)
...ain/java/htsjdk/samtools/cram/io/CramIntArray.java	88.889% <0%> (ø)	4% <0%> (ø)	⬇️
...ava/htsjdk/samtools/cram/structure/EncodingID.java	100% <0%> (ø)	3% <0%> (+2%)	⬆️
...sjdk/samtools/cram/structure/BlockContentType.java	100% <0%> (ø)	3% <0%> (+2%)	⬆️
...amtools/cram/structure/BlockCompressionMethod.java	100% <0%> (ø)	3% <0%> (+2%)	⬆️
... and 14 more

[lbergelson]

@Wathis Thank you! Looks good 👍