Return correct subclass from relationship

[pvanheus]

I have a model with this design: Gene is located on either Chromosome or Contig. At the moment I'm solving this in the model by creating a class Feature from which both Chromosome and Contig inherit. The RelationshipTo is restricted to specifying a single class, so node I have code similar to:

class Feature(StructuredNode):
    name = StringProperty()
    location = RelationshipTo('Feature', 'LOCATED_ON', cardinality=One)

class Gene(Feature):
    description = StringProperty()

class Chromosome(Feature):
    pass

class Contig(Feature):
    pass

In the real model, Contig and Chromosome have different attributes. The problems that arise with this model are that:

1. A spurious label (Feature) is introduced into the database.
2. When querying for location (gene.location.all()) a Feature is returned, not the actual Chromosome or Contig that the gene is located on.

Inside the code the correct label is identified during querying but the actual object returned is of the superclass.

[robinedwards]

Hmm if you add __abstract_node__ to Feature it will solve the issue. Presuming Feature is abstract? The behaviour is a bit strange though and needs to be fixed

[pvanheus]

There are a number of bugs related to this.

1. If you specify that Feature is an __abstract_node__ and then set a cardinality=One attribute on the location relationship, you get this error:

/home/pvh/anaconda3/bin/python /home/pvh/Documents/code/SANBI/neo4j_notebook/neo4j_experiments.py
Traceback (most recent call last):
  File "/home/pvh/Documents/code/SANBI/neo4j_notebook/neo4j_experiments.py", line 195, in <module>
    thing.location.connect(chrom)
  File "/home/pvh/anaconda3/lib/python3.5/site-packages/neomodel/cardinality.py", line 124, in connect
    if len(self):
  File "/home/pvh/anaconda3/lib/python3.5/site-packages/neomodel/relationship_manager.py", line 309, in __len__
    return self._new_traversal().__len__()
  File "/home/pvh/anaconda3/lib/python3.5/site-packages/neomodel/match.py", line 446, in __len__
    return self.query_cls(self).build_ast()._count()
  File "/home/pvh/anaconda3/lib/python3.5/site-packages/neomodel/match.py", line 220, in build_ast
    self.build_source(self.node_set)
  File "/home/pvh/anaconda3/lib/python3.5/site-packages/neomodel/match.py", line 231, in build_source
    return self.build_traversal(source)
  File "/home/pvh/anaconda3/lib/python3.5/site-packages/neomodel/match.py", line 275, in build_traversal
    rhs_label = ':' + traversal.target_class.__label__
AttributeError: type object 'Feature' has no attribute '__label__'

What is going on here is that One is calling len() which calls __len__() on RelationshipManager which in turn calls _new_traversal() which ultimately leads to the error because in the definition of the traversal the target class is Feature which has no label.

If you don't use the cardinality attribute you're left with problem 2 mentioned above.

[pvanheus]

Thinking about this some more... I think the correct solution is that you need to keep the Feature labels to enable queries to work, but when you inflate the resulting objects from the data, you need to inflate to the subclass mentioned in the subclass label. I'm working on some code to test this approach.

[pvanheus]

@robinedwards The idea behind #263 is to apply this logic:

1. If a node has multiple labels one of those labels might be a valid subclass of the class in the traversal's description.
2. We can find the subclasses of the class mentioned in the traversal's description by walking the correct Python datastructure. So we can build a list of these.
3. If one of these intersects with the list of labels, we should use that class as the class of node to create.
4. If more than one of them intersects with the list of labels we don't know what to do. This is an error.

This logic works for me. I think it is valid in the general case. Perhaps enabling this could be a config option?

[robinedwards]

Hey apologies my brain wasn't in gear yesterday.

We need to be a little bit careful as I think there is some existing functionality built around assuming abstract nodes dont have labels.. However your case above (relation to abstract node) should definitely raise a friendly error message in the current release of neomodel.

What you are trying to achieve is quite similar to #126

Now I am not opposed to changing the behaviour of inheritance in a major release of neomodel, the current situation is confusing at best. In fact the whole handling of 'abstract nodes' is hacky and unpythonic. One issue with the subclass behaviour is when someone wants to build a model which actually points to the super class (none abstract) it becomes tricky.

[aanastasiou]

@pvanheus Can I please ask if you have tried version 3.3.1? The changes there would cover this use-case I wonder if this would be enough to close this issue (?).