Make --taxonomy select the priors it names - #6
Merged
Conversation
lineage_paths() required One_Hop_Refilled_*.json and Two_Hop_Refilled_*.json under ONE_HOP_NEIGHBOR_DATA and TWO_HOP_NEIGHBOR_DATA. Neither directory is present, and it looks like neither was ever part of the code, so the check failed for every taxonomy including the default and every run loaded domain priors. The two paths were returned and never read, so nothing failed. Four runs of the 0.1.5 container differing only in -t produced byte identical output. Check only the counts table and the neighbour graph, the two files the code reads. The accepted names now come from the installed files rather than a hardcoded list. The lists they replace had drifted three ways: they named "cyanobacteriota" while the file is "cyanobacteriota_melainabacteria_group", omitted "fusobacteriota", and filed "mycoplasmatota" as a kingdom though the -ota suffix makes it a phylum. Two prior sets stayed unreachable even after that check was corrected. A name may be shortened where exactly one canonical name extends it at an underscore, so "cyanobacteriota" and "fcb" resolve while "cyano" does not. An unrecognized name causes an error that lists the valid names by rank, or the near misses when the input is close to one. It used to select domain priors in silence. The domain-level taxon is called bacteria, since every other entry in the list is a taxon rather than a rank. Logging reports the priors in use instead of the string the user typed. Those disagreed: asking for cyanobacteriota logged cyanobacteriota while loading the domain file. tests/taxonomy_test.py runs one input under four taxonomies and requires the results to differ. It fails against the current release. Implemented with assistance from Claude (Opus 5)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was broken
lineage_paths()requiredOne_Hop_Refilled_*.jsonandTwo_Hop_Refilled_*.jsonunderONE_HOP_NEIGHBOR_DATAandTWO_HOP_NEIGHBOR_DATA. Neither directory is present, and it looks like neither was ever part of the code. The check therefore failed for every taxonomy, including the default, and every run loaded domain-level priors. The two paths were returned and never read, so nothing failed and nothing appeared in the log beyond a note on stderr.Four runs of the released 0.1.5 container on one input, differing only in
-t:actinomycetota_phylum_level_priorsships under exactly that name, with no mismatch, and it still fell back. That isolates the missing directories as the cause.Three more problems in the same code
The hardcoded
PHYLUMandKINGDOMsets had drifted from the files that ship:cyanobacteriotawas listed, but the file iscyanobacteriota_melainabacteria_group_phylum_level_priorsfusobacteriota_phylum_level_priorsships but appears in neither setmycoplasmatotawas listed as a kingdom, though the-otasuffix makes it a phylumThose two prior sets stayed unreachable even once the file check was corrected.
What changed
The accepted names are read from the installed files instead of a hardcoded list, so they cannot disagree with what ships. The scan matches the filename pattern rather than listing the directory, since a
README.mdsits in both data directories.A name may be shortened where exactly one canonical name extends it at an underscore, so
cyanobacteriotareaches the melainabacteria group set andfcbreachesfcb_group, whilecyanois refused.An unrecognized name now causes an error. It lists the valid names grouped by rank, or names the near misses when the input is close to one:
The domain-level taxon is called
bacteria, since every other entry in the list is a taxon rather than a rank.domainstill works as input.Logging reports the priors in use rather than the string the user typed. Those used to disagree: asking for
cyanobacteriotaloggedcyanobacteriotawhile loading the domain file.The one-hop and two-hop paths are removed from
Pathsand from the return value, since nothing read them.Breaking change for anyone passing -t
A run that passed
-tand silently got domain priors will now either produce different numbers, or stop with an error if the name is not recognized. Both are the intended fix, but results generated with-tbefore this change are not comparable to results generated after it. The 0.1.6 release notes should say so.Results produced without
-tare unaffected, since those were domain priors either way.Test
tests/taxonomy_test.py, wired into CI, runs one input under four taxonomies and requires the results to differ. It fails against the current release. On this branch:It also checks that all 20 names round-trip, that every
-atiname is a kingdom and every-otaname a phylum, that the two previously unreachable sets are reachable, and that an invalid name exits non-zero.