search

dice-group / owlapy

OWLAPY is a Python Framework for creating and manipulating OWL Ontologies.
MIT License
16 stars 1 forks source link

Using Sync as a prefix #79

Open Demirrr opened 2 days ago

Demirrr commented 2 days ago

What is the purpose of using Sync as a prefix for few classes ?

alkidbaci commented 2 days ago

Those classes are using owlapi for their functionalities and the idea of "Sync" is that those are synchronizing classes in a sense that nothing is actually implemented in our package and when you are calling a method in python, the actual processing is happening in some java library. Basically its "syncing" our framework with another framework. This naming is not my original idea and is based on a previous naming in owlapy which I believe is based on the naming in owlready2 for a similar functionality.

Demirrr commented 2 days ago

Yes I thought so. I guess we can start thinking about omitting this prefixing.

I guess we need some refactroing in the owl_reasoner.py

  1. Move AbstractOWLReasonerEx into AbstractOWLReasoner (unclear why these inconvenience needed to be seperated)
  2. Delete OntologyReasoner class and
  3. Implement OWLReasoner class

A reasoner should be created as reasoner= OWLReasoner(reasoner="Hermit | Pellet .... | FastInstanceCheckerReasoner") We might even need to rename FastInstanceCheckerReasoner.

Do you think such refactoring would make our interface better ?

alkidbaci commented 13 hours ago

For point 1, I agree it is better to merge them as we will always want a reasoner to use methods of both classes.

For point 2 I also agree. To add a bit of context on this, as you may already know, OntologyReasoner is kind of incomplete because the instances method does not work for complex class expressions and there comes the need for FastInstanceCheckerReasoner which is basically the same as OntologyReasoner but with support for complex CEs and some caching taking place which makes things 'faster'. We can make FastInstanceCheckerReasoner the default one, and probably rename it.

As for the third point and the idea of merging everything under one class name does not seem optimal to me. In our framework we encourage an architecture where someone can implement different reasoners based on the AbstractOWLReasoner class. And so we have done. For example FastInstanceCheckerReasoner and SyncReasoner are very different ones, because the former operates under CWA (or partial CWA, still not clear where it stands) and the latter under OWA. We also have other reasoners in ontolearn like TripleStoreReasoner or Luke's neural reasoner. I understand that we can still have this distinction if we implement the suggested OWLReasoner class but we cannot put in the same choice list 'HermiT'/'Pellet'/... and FastInstanceCheckerReasoner. There is also the difference in libraries used to implement those reasoners. Therefore, i think its better to leave the structure as is.

Another thing you point out is naming. The reasoner-related classes has gone through some heavy renaming over the past few months and i feel like they are losing their significance. I believe we should think this more through and come up with a naming/arranging strategy that we wont re-evaluate again in the near future. OWLReasoner once used to be an abstract class. Therefore, my suggestion is as following: We don't use the name OWLReasoner and we name the reasoners based on their main characteristic as we already are doing. As for FastInstanceCheckerReasoner I believe, as you also suggested, that renaming is needed. This time i would say to use simple to remember name. As long as we have clear definitions in the documentation it would be easy for everyone to find the appropriate reasoner to use. The reason i dont want to use the name OWLReasoner in this scenario is because its very general, its like calling a human being "Human". We want to have "Alex", "Bob", "Mary" (just to stick at the human <=> reasoner example). Again, I understand that this can work the way you suggested it but why say Human("Alex") when we can just say "Alex".

If everything sounds good to you then I would continue to work on the following TODOs: