phylo42 / IPK

Inference of phylo-k-mers
MIT License
4 stars 1 forks source link

Newick parser bug #13

Open nromashchenko opened 1 year ago

nromashchenko commented 1 year ago

If a clade name contains symbols like "(", ")", the parser crashes assuming that it's part of the tree syntax. Name example:

... ('AE006468, Salmonella enterica subsp. enterica, Salmonella enterica subsp. enterica, (AE006468), LT2 T':0.01112,...

blinard-BIOINFO commented 1 year ago

@nromashchenko, as a general trend in what i see in bioinformatic programs, people solve the issue by a) forbidding these characters in the sequence labels . Or b) by replacing all : and () characters by _ . First in the alignement, then in the newick by exact match of the original alignment label. (I think I used the second in RAPPAS).

lczech commented 1 year ago

Hey there,

the (not super well standardized) Newick specification allows that, as long as it's in quotes (single - but I vaguely remember that I've seen double quotes as well somewhere...). See also https://doi.org/10.1093/molbev/msx055 for the mess of Newick.

Also, as for @blinard-BIOINFO's comment: Yes, that seems to be the case, many tools go that way now. But personally, I find it an unfortunate trend that often file formats are not properly implemented. Sure, in this case at least it will fail somehow ("crashing" probably not being the best way though). There are however also cases where tools will silently continue, such as when requiring FASTA files to have exactly 2 lines per record, instead of allowing line breaks in the sequence. But given that bioinformatics file formats are a huge mess anyway, I think it will only get messier if tools require their own simplified versions, shifting the burden to the user to check and convert as needed.

Don't mean to rant here - it's just something that has bugged me in too many tools, and hence wanted to use this opportunity to advocate to properly implement the format :-)

Cheers Lucas