search

simphony / simphony-osp

A framework that aims to achieve interoperability between software such as simulation engines, databases and data repositories using a knowledge graph as the common language.
https://simphony.readthedocs.io
Other
16 stars 12 forks source link

Unable to access ontology classes whose names contain hyphen #562

Closed yoavnash closed 3 years ago

yoavnash commented 3 years ago

Protege allows to name an entity with a hyphen, like so: Pre_Post-Processing_Atomistic. However, in the python code, this oclass will not be accessible because hyphens are not allowed according to python's syntax rules.

urbanmatthias commented 3 years ago

I think getitem should be implemented for namespaces. So maybe you can try to query it like a dictionary.

kysrpex commented 3 years ago

Indeed, it is possible to query the namespace as a dictionary, but actually the query returns a list. We should make it return the OntologyClass directly.

from osp.core.namespaces import perceptual  # From the EMMO Ontology
query_result = perceptual['1-manifold']
print(type(query_result), query_result)
<class 'list'> [<OntologyClass perceptual.1-manifold>]
urbanmatthias commented 3 years ago

Ah I remember. With the square brackets, one will query by label. Since it is theoretically possible to have multiple classes with the same label, I decided to return a list.

We could assume there are no duplicate labels, but I guess there should be some fallback method for the case where there are duplicate labels.

kysrpex commented 3 years ago

I have chosen to return just one element when using square brackets, instead of a list. Now an error will be raised when a query by label returns more than one element with the same label. The solution is almost finished, and I will commit it soon.

I like this idea, but I would like to have your opinion also on using a warning instead. In such a case, maybe the first element found could be returned, showing its IRI on the warning. I am thinking of very long simulations where all the computation time goes to waste. Maybe it would be better to show through the warning that the results may not be trustworthy and letting the user decide that, instead of forcing the process to be interrupted. Definitely I would still be against keeping the list behavior, as the results would not be as "trustworthy" as in the error on warning cases.

In the meantime, while this solution makes it to master, have you tried getattr(namespace_object, 'Pre_Post-Processing_Atomistic') as a workaround? This might work.

pablo-de-andres commented 3 years ago

Would it make sense to check that there are no duplicate labels when installing the ontology? In that case an error wouldn't be so disrupting. And we can check other constraints that we end up applying to the ontology. Then you are sure the case won't happen

kysrpex commented 3 years ago

Would it make sense to check that there are no duplicate labels when installing the ontology? In that case an error wouldn't be so disrupting. And we can check other constraints that we end up applying to the ontology. Then you are sure the case won't happen

Good idea, so if two labels coincide on installation we may prevent referencing the namespace by label, and only allow suffix/IRI.