search

biolink / ontobio

python library for working with ontologies and ontology associations
https://ontobio.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
118 stars 30 forks source link

param to run ALL GO rules in ontobio-parse-assocs does not run all rules #558

Closed dustine32 closed 3 years ago

dustine32 commented 3 years ago

Related to #544

@dougli1sqrd Was having some trouble getting all rules to run using the new -l all param. I had a WB GPAD with two lines annotated to CC root but they used relation located_in, which violates GO rule 61. These didn't get repaired when running ontobio-parse-assocs.py with the -l all param.

Throwing on my debugger, I traced it to this line in assocparser.py: https://github.com/biolink/ontobio/blob/d4fff4a551b71d15eb3be2f8296816ee2f721ca0/ontobio/io/assocparser.py#L268 Here, rule_set was coming in as list ["all"] but RuleSet.ALL is just the str "all": https://github.com/biolink/ontobio/blob/d4fff4a551b71d15eb3be2f8296816ee2f721ca0/ontobio/io/assocparser.py#L193 To fix, I just "listified" RuleSet.ALL in the conditional:

elif rule_set == [RuleSet.ALL]:

@dougli1sqrd I can PR this change and you can trash it if you don't like it. Thanks!

dougli1sqrd commented 3 years ago

Oh good catch. The reason, though, this is coming in as a list is because the command line option in ontobio-parse-assocs is being created as a list, so "all" is being inserted as an item.

My thought is that since this is an artifact of the argument parsing, we should parse that there in ontobio-parse-assocs. If the incoming args.rule_set contains RuleSet.ALL then we should just set the config to be RuleSet.ALL rather than a list, as expected by the logic in the config init.

dustine32 commented 3 years ago

@dougli1sqrd Thanks! I just corrected it in the PR.