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

Behavior of get method for non-active relationships #424

Open yoavnash opened 4 years ago

yoavnash commented 4 years ago

Currently, the default behavior of the get only returns objects that are linked via an active relationship. This could lead to a surprising/unintuitive behavior, especially for non-containment relationships.

Example:

alice = my_onto.person(name="Alice")
bob = my_onto.person(name="Bob")
alice.add(bob, rel=my_onto.friends_with) # friends_with is a non-containment (and symmetrical) relationship

# more code ...

# get Alice's friends:
bob_retrieved = alice.get(bob.uid)
# bob_retrieved is equal to None, although Alice is still friends with Bob.

Proposed solution 1

Extend the behavior of the get method so that it also returns object linked with non-containment relationships, while still ignoring those that are linked in a passive relationship. In other words, the default relationship of get should not be active_relationship, but rather the complement of passive_relationship.

Proposed solution 2

Extend the behavior of the get method so that it returns all connected objects. This is more intuitive in the sense that here get() is more general than get(rel=onto.is_part_of) as one might expect (it's not the case with the current behavior). Possible downside: this change will perhaps encourage less thinking in terms of containers, which is a central idea in OSP-core.

pablo-de-andres commented 4 years ago

will it really be natural for the enduser to call get(X) for all X related through non-passive relationships, but have to explicitly state when it is passive?

I'm not sure the get should not just work for all the related objects. If we expand it to work for non-containment relationships, we would be saying it works for all types of related objects, except the reverse containment.

What if I have a Position object and I want to get the atom it is connected to? position.get(oclass=Atom) would also raise an exception (or not return anything)?

yoavnash commented 4 years ago

In my opinion, if the user knows about containment, then it's easier to predict the behavior of the get method for passive relationships. However, for non-containment relationships the current behavior is clearly unintuitive.

yoavnash commented 4 years ago

I added a second possible solution. For me, perhaps I like a bit more solution 2 because it's more simple.

urbanmatthias commented 4 years ago

I agree that solution two is the simplest. However it feels like we are - step by step - loosing the container characteristic of the CUDS objects.