search

owlcollab / owltools

OWLTools
BSD 3-Clause "New" or "Revised" License
108 stars 33 forks source link

Command --solr-load-models unecessarily fails when there is no provided "lego" catalog #232

Closed kltm closed 6 years ago

kltm commented 6 years ago

While it seemed like a good idea to require it when we wrote it, we would now prefer that "lego" catalogs are optional for the --solr-load-models command.

kltm commented 6 years ago

The fix should be as simple as removing the test on line 559 and ensuring that legoCatalogs is a string array. (And remove specifying error test at 562). Will work on PR in a bit...

kltm commented 6 years ago

A step forward, a step back. Testing the loader, we now fail with:

2018-01-18 00:14:59,213 INFO  (ParserWrapper:82) Finished loading ontology: http://model.geneontology.org/0000000300000001 from: file:/home/bbop/local/src/git/noctua-models/models/0000000300000001.ttl
2018-01-18 00:15:45,210 ERROR (SolrCommandRunner:609) Unsatisfiable: 0000000300000001.ttl == [<http://purl.obolibrary.org/obo/EMAPA_36026>, <http://purl.obolibrary.org/obo/EMAPA_25090>, <http://purl.obolibrary.org/obo/EMAPA_16777>, <http://purl.obolibrary.org/obo/EMAPA_16133>,
...

This is understandable as (IIRC) it is part of a set of mock-up models--I doubt anything sensible would be loaded even it was satisfiable. I think the best course of action would be to merge in a PR for d35e598 and open a new ticket for this, with the desired outcome to continue on with other models if any single model unsatisfiable. Any thoughts @cmungall ? Maybe just wait until we have an alternate loader (although I would like to have something in there soon for demo purposes and this would be the fastest way forward)?

cmungall commented 6 years ago

Yes, let's load all coherent models and report incoherent ones. Let's not wait on a new loader.

it is odd that this model is incoherent through unsatisfiable classes. It's hard for an abox model to force an unsatisfiable class, only to introduce an inconsistency. But we can investigate this later.

kltm commented 6 years ago

Likely fixed by #233 (review @cmungall)