Merge TripleStoreKnowledgeBase and TripleStore

[Jean-KOUAGOU]

I have implemented a simple merge of TripleStore and TripleStoreKnowledgeBase. Tests work locally. Please review and merge

[Demirrr]

You didn't really merge TripleStoreKnowledgeBase and TripleStore but rather put already defined class of TripleStoreKnowledgeBase into TripleStore. What is the benefits of this ?

[Demirrr]

Remove
if name == "main":
test_triplestore = TestTriplestore()
test_triplestore.test_remote_triplestore_dbpedia_tdl()

No need these three lines.

[Demirrr]

No need to print

[Demirrr]

pass is problematic. This is not a really test

[Demirrr]

This is incorrect. We cannot have TripleStoreOntology, TripleStoreReasoner, and TripleStoreReasonerOntology, can we ?

[Jean-KOUAGOU]

I thought merging was putting them together so we don't have two defined classes. I did not define new functions, but merged already existing ones.

…

On Fri, Nov 29, 2024, 11:22 Caglar Demir ***@***.***> wrote: ***@***.**** requested changes on this pull request. You didn't really merge TripleStoreKnowledgeBase and TripleStore but rather put already defined class of TripleStoreKnowledgeBase into TripleStore. What is the benefits of this ? ------------------------------ In tests/test_triplestore.py <#497 (comment)>: > assert h assert DLSyntaxObjectRenderer().render(h) save_owl_class_expressions(h) sparql = Owl2SparqlConverter().as_query("?x", h) assert sparql else: pass - """ + Remove if *name* == "*main*": test_triplestore = TestTriplestore() test_triplestore.test_remote_triplestore_dbpedia_tdl() No need these three lines. ------------------------------ In tests/test_triplestore.py <#497 (comment)>: > @@ -53,12 +52,16 @@ def test_remote_triplestore_dbpedia_tdl(self): typed_neg = set(map(OWLNamedIndividual, map(IRI.create, examples["negative_examples"]))) lp = PosNegLPStandard(pos=typed_pos, neg=typed_neg) h = model.fit(learning_problem=lp).best_hypotheses() + print(h) No need to print ------------------------------ In tests/test_triplestore.py <#497 (comment)>: > @@ -53,12 +52,16 @@ def test_remote_triplestore_dbpedia_tdl(self): typed_neg = set(map(OWLNamedIndividual, map(IRI.create, examples["negative_examples"]))) lp = PosNegLPStandard(pos=typed_pos, neg=typed_neg) h = model.fit(learning_problem=lp).best_hypotheses() + print(h) assert h assert DLSyntaxObjectRenderer().render(h) save_owl_class_expressions(h) sparql = Owl2SparqlConverter().as_query("?x", h) assert sparql else: pass pass is problematic. This is not a really test ------------------------------ In ontolearn/triple_store.py <#497 (comment)>: > + def __init__(self, url: str=None): + assert url is not None, "url must be string" + self.g = TripleStoreReasonerOntology(url=url) This is incorrect. We cannot have TripleStoreOntology, TripleStoreReasoner, and TripleStoreReasonerOntology, can we ? — Reply to this email directly, view it on GitHub <#497 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AK4X4ERYEY6CRNJYVD6ODRT2DA57DAVCNFSM6AAAAABSOIIVOGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDINRZGU3TQOBRGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[Demirrr]

@alkidbaci could you please review this PR ? Important that the existing merged class should work seamlsy with TDL, DRILL and others. Please note that it is important that TDL and DRILL work without a problem.

[alkidbaci]

I noticed that TripleStore class is now inheriting from KnowledgeBase which is against the main reason we have TripleStore class in the first place. Because of that we have to set the reasoner and the ontology of that class, and that's why we have TripleStoreOntology, TripleStoreReasoner and TripleStoreReasonerOntology at the same class.
It's kind of the right direction but not quite the right way.

TDL and Drill works. Drill tried only on family dataset.

These changes can be considered temporary but I suggest we have a more long lasting solution for this problem.