Illumina / REViewer

A tool for visualizing alignments of reads in regions containing tandem repeats
GNU General Public License v3.0
73 stars 14 forks source link

JSON metrics output #36

Closed egor-dolzhenko closed 2 years ago

egor-dolzhenko commented 2 years ago

This pull request will:

Example:

{
  "DMPK": {
    "AlleleDepths": "28.50/21.93",
    "Genotype": "12/112"
  }
}
ctsa commented 2 years ago

Do we want to force the metrics users to string parse these outputs? Why shouldn't these per-allele values be a list instead of a string separated by "/"?

egor-dolzhenko commented 2 years ago

Do we want to force the metrics users to string parse these outputs? Why shouldn't these per-allele values be a list instead of a string separated by "/"?

Good question. The pros of this format is consistency with EH output and that some users work with these jsons directly in their text editors:

{
  "DMPK": {
    "AlleleDepths": "28.50/21.93",
    "Genotype": "12/112"
  }
}

might be a bit simpler to read than this (especially as the number of metrics grows)

{
   "DMPK":{
      "AlleleDepths":[
         28.50,
         21.93
      ],
      "Genotype":[
         12,
         112
      ]
   }
}

As you pointed out, the con is that this format is harder to parse. Perhaps it's a good enough reason to switch to it. What do you think?

ctsa commented 2 years ago

In general, one of the big points of VCF->JSON is that parsing becomes trivial, so I don't think the readability is really a consideration, but consistency with EH is really important here, I agree for that reason it probably should be left as is.

egor-dolzhenko commented 2 years ago

Sounds good!