iqbal-lab / Mykrobe-predictor

Antibiotic resistance predictions in minutes on a laptop
Other
50 stars 19 forks source link

Harmonise JSON output between Staph and TB #37

Closed iqbal-lab closed 9 years ago

iqbal-lab commented 9 years ago

Spoke to Simon. Simplest to make final release changes to Mykrobe UI if have same JSON format for both.

Phelimb commented 9 years ago

OK, good to know. Will work on it.

Phelimb commented 9 years ago

Can I propose we kill stdout altogether while we do this and make json mandatory? I haven't been keeping it up to date and I fear it's a bit of a mess.

iqbal-lab commented 9 years ago

Agreed. And remove timestamps?

Phelimb commented 9 years ago

I think timestamps are stdout only anyway so removing the option will also remove timestamps.

Phelimb commented 9 years ago

Shall we include a version string in the JSON ?

Phelimb commented 9 years ago

Updated JSONs.

Staph

{
    "expected_depth": "77",
    "mean_read_length": "48",
    "phylogenetics": {
        "complex": {
            "Staphylococcus": "82"
        },
        "species": {
            "S. aureus": "82"
        },
        "lineage": {
            "N/A": "-1"
        }
    },
    "susceptibility": {
        "Gentamicin": "S",
        "Penicillin": "R",
        "Methicillin": "R",
        "Trimethoprim": "S",
        "Erythromycin": "R",
        "FusidicAcid": "S",
        "Ciprofloxacin": "R",
        "Rifampicin": "S",
        "Tetracycline": "S",
        "Vancomycin": "S",
        "Mupirocin": "S",
        "Clindamycin": "R(inducible)"
    },
    "called_variants": {
        "gyrA_S84L": {
            "R_per_cov": "100",
            "S_per_cov": "0",
            "R_median_cov": "101",
            "S_median_cov": "0",
            "conf": "1092",
            "induced_resistance": "Ciprofloxacin"
        }
    },
    "called_genes": {
        "blaZ": {
            "per_cov": "74",
            "median_cov": "82",
            "conf": "42298",
            "induced_resistance": "Penicillin"
        },
        "ermC": {
            "per_cov": "99",
            "median_cov": "834",
            "conf": "2496",
            "induced_resistance": "Erythromycin"
        },
        "mecA": {
            "per_cov": "99",
            "median_cov": "96",
            "conf": "285",
            "induced_resistance": "Methicillin"
        }
    },
    "virulence_toxins": {
        "PVL": "negative"
    }
}

TB

{
    "expected_depth": "58",
    "mean_read_length": "91",
    "phylogenetics": {
        "complex": {
            "MTBC": "100"
        },
        "species": {
            "M. tuberculosis": "63"
        },
        "lineage": {
            "Delhi/Central Asia": "79"
        }
    },
    "susceptibility": {
        "Isoniazid": "S",
        "Rifampicin": "S",
        "Ethambutol": "S",
        "Pyrazinamide": "S",
        "Quinolones": "S",
        "Streptomycin": "S",
        "Amikacin": "S",
        "Capreomycin": "S",
        "Kanamycin": "S"
    },
    "called_variants": {},
    "called_genes": {},
    "virulence_toxins": {}
}
iqbal-lab commented 9 years ago

Version string is good. Commit would be ideal, but hard to organise

iqbal-lab commented 9 years ago

Don't understand what the bug was in calling non-aurues staphs?

iqbal-lab commented 9 years ago

Whats the complex for Staph? If we want to mirror what we do in TB, where the complexes are the two groups we care about, then one complex is S. aureus and the other is coag-neg, right?

Phelimb commented 9 years ago

Bug was that we never called them because we masked them all out rather than masking out aureus. Fixed now.

I can change staph to say aureus or coag neg. I think from a code level though it's not the exact equivalent since we look for catalase to call it as Staph and then have species panels.

Phelimb commented 9 years ago

Sorry, that bug commit is confusing. The bug fix is only in src/predictor/staph/species.c a tiny change. The rest is removing stdout which I should have done in another commit.

iqbal-lab commented 9 years ago

I see. So the question is - what is complex for. My proposal is we use it to decide if we should print resistance predictions. For Staph you print them if there is any aureus, and for TB you do so if there is any MTBC. What do you think?

Phelimb commented 9 years ago

Yes, sounds good. In the case of pure aureus it just seemed strange to me to have complex : auerus, species : aureus.

iqbal-lab commented 9 years ago

yes, it is weird. We could change it from Complex to In-group and Out-group?

iqbal-lab commented 9 years ago

Oh no, needs to change from Complex to one word, not two options!

iqbal-lab commented 9 years ago

SpeciesGroup?

Phelimb commented 9 years ago

Or Clade?

iqbal-lab commented 9 years ago

I basically don't know properly what clade means. I think clade corresponds to a subtree. We want a word that signifies we have split our "local phylogeny" (eg Staphylococci) into two (aureus and not-aureus). How about Partition?

On 12 February 2015 at 22:51, Phelim Bradley notifications@github.com wrote:

Or Clade?

— Reply to this email directly or view it on GitHub https://github.com/iqbal-lab/myKrobe-predictor/issues/37#issuecomment-74170969 .

Phelimb commented 9 years ago

To be honest, we don't really need it at all for Staph. We didn't have it before and the only reason I added it was to be consistent with TB.

Phelimb commented 9 years ago

So whatever word would need to work for MTBC and NTM too otherwise there's not much point .

iqbal-lab commented 9 years ago

Yes, true. Maybe just Group is safe (if boring). We could leave it empty for Staph I suppose, but I thin Aureus versus CoagNeg works

iqbal-lab commented 9 years ago

This is not worth any more of our time.

iqbal-lab commented 9 years ago

Phylogroup?

Phelimb commented 9 years ago

:+1:

Phelimb commented 9 years ago

Tested on staph species with no change so merged to master.