Implement new link types#2220
Merged
sphuber merged 4 commits intoNov 26, 2018
Merged
Conversation
sphuber
force-pushed
the
fix_2177_implement_new_link_types
branch
from
bf4458c to
c3ca6e4
Compare
With the new ORM hierarchy in place, the corresponding link types can be defined: CREATE: CalculationNode -> Data RETURN: WorkflowNode -> Data INPUT_CALC: Data -> CalculationNode INPUT_WORK: Data -> WorkflowNode CALL_CALC: WorkflowNode -> CalculationNode CALL_WORK: WorkflowNode -> WorkflowNode These rules are encoded in the `validate_link` function in `aiida.orm.utils.links`. This function is called from `add_link_from`, which will be renamed to `add_incoming`, and validate the triple (source, link, target) representing the addition of a link from a source node to a target node.
sphuber
force-pushed
the
fix_2177_implement_new_link_types
branch
3 times, most recently
from
2de265a to
51ff0bc
Compare
This is to align it with the recent implementation of `get_incoming` and `get_outgoing` to obtain all incoming and outgoing links, respectively.
sphuber
force-pushed
the
fix_2177_implement_new_link_types
branch
from
51ff0bc to
9db7997
Compare
muhrin
suggested changes
Nov 23, 2018
Contributor
There was a problem hiding this comment.
Consider changing to just check the link labels for uniqueness at this point.
Contributor
There was a problem hiding this comment.
Consider using self.assertClickResultNoException
Contributor
There was a problem hiding this comment.
Add a case where link label is different
Contributor
There was a problem hiding this comment.
Leave checks out from backend classes
Contributor
There was a problem hiding this comment.
Is this needed? Not already in Node?
Contributor
Author
There was a problem hiding this comment.
I remember now, this needs to be in the subclasses Data, CalculationNode and WorkflowNode, because the tests currently, for simplicity, use base Nodes to create dummy graphs. If I were to put these checks in the Node base class, the type checks would fail. Until we rewrite the tests to only ever use Data, CalculationNode and WorkflowNode instances to create dummy graphs, this has to stay in the sub classes.
sphuber
force-pushed
the
fix_2177_implement_new_link_types
branch
2 times, most recently
from
58f8ed6 to
ff8ba5b
Compare
The link triple is defined as the tuple of the source node, link type and link label of an incoming link into a target node. This concept is implemented in a named tuple `LinkTriple`. Since the introduction of link types, the link label of incoming links no longer necessarily needs to be unique, as long as the triple with the link type and source node *is* unique. However, the old implementation of the incoming link cache relied on label uniqueness, because it used only the label as the key in the cache, which used a dictionary as its data structure. Here we change that data structure of the node's internal incoming link cache to be a set of link triples.
sphuber
force-pushed
the
fix_2177_implement_new_link_types
branch
6 times, most recently
from
ddc18d0 to
1d94bad
Compare
The `link_label` used to be optional when specifying a link, as long as both nodes were already stored. If one of the nodes was unstored the link had to be stored in the internal cache which was indexed on the link label. This was already changed to be indexed on the link triple instead, solving that issue, but the auto label generation in `Node._add_dblink_from` in the case of no label being defined, relied on a uniqueness constraint of the link label on the database. This constraint had already been removed a long time ago when the `RETURN` link type was introduced. However, the logic of the auto label generation was kept, even though it would never work. We remove that functionality here and make the explicit passing of a link label required in the `add_incoming` and `_add_dblink_from` methods.
sphuber
force-pushed
the
fix_2177_implement_new_link_types
branch
from
1d94bad to
834e27a
Compare
muhrin
approved these changes
Nov 26, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2177
With the new ORM hierarchy in place, the corresponding link types can be defined:
CREATE:CalculationNode -> DataRETURN:WorkflowNode -> DataINPUT_CALC:Data -> CalculationNodeINPUT_WORK:Data -> WorkflowNodeCALL_CALC:WorkflowNode -> CalculationNodeCALL_WORK:WorkflowNode -> WorkflowNodeThese rules are encoded in the
validate_linkfunction inaiida.orm.utils.links.This function is called from
add_link_from, which has been renamed toadd_incoming,and validates the triple (source, link, target) representing the addition of a link
from a source node to a target node.