Skip to content

Further simplification of node type string definition and loading#2401

Merged
giovannipizzi merged 1 commit into
aiidateam:provenance_redesignfrom
sphuber:fix_1603_node_class_loading
Jan 19, 2019
Merged

Further simplification of node type string definition and loading#2401
giovannipizzi merged 1 commit into
aiidateam:provenance_redesignfrom
sphuber:fix_1603_node_class_loading

Conversation

@sphuber
Copy link
Copy Markdown
Contributor

@sphuber sphuber commented Jan 18, 2019

Fixes #1603

With the recent introduction of the CalcJob process, which no ensures
that also for job calculations, it is the CalcJobNode that gets stored
in the database, which has a type string just like all the other node
types. This made a lot of the old logic aiida.plugins.loader to
determine the correct type string for a Node (sub) class obsolete as
now all node classes are per definition internal to aiida-core and
have a type string that is based directly on their module path.

The few functions that remain, get_type_string_from_class that formats
the correct type string for a given Node class and the
load_node_class which can reload that class from the string have been
moved to aiida.orm.utils.node since it has no longer anything to do
with a "plugin".

Finally, by moving the WorkflowNode, CalculationNode and
ProcessNode classes in the __init__ files of their respective
modules allows us to add further simplification to load_node_class as
well as making all these classes and their subclasses importable from
the aiida.orm.node module. Eventually, when aiida.orm.data is also
moved there, all classes can even be exposed directly on aiida.orm and
that is the deepest a user should ever go for they should need or be
allowed to import.

@coveralls
Copy link
Copy Markdown

coveralls commented Jan 18, 2019

Coverage Status

Coverage decreased (-0.01%) to 69.606% when pulling 542a3b6 on sphuber:fix_1603_node_class_loading into c0fba2e on aiidateam:provenance_redesign.

@sphuber sphuber force-pushed the fix_1603_node_class_loading branch 3 times, most recently from 54e8082 to 5a26753 Compare January 18, 2019 17:34
With the recent introduction of the `CalcJob` process, which no ensures
that also for job calculations, it is the `CalcJobNode` that gets stored
in the database, which has a type string just like all the other node
types. This made a lot of the old logic `aiida.plugins.loader` to
determine the correct `type` string for a `Node` (sub) class obsolete as
now all node classes are per definition internal to `aiida-core` and
have a type string that is based directly on their module path.

The few functions that remain, `get_type_string_from_class` that formats
the correct type string for a given `Node` class and the
`load_node_class` which can reload that class from the string have been
moved to `aiida.orm.utils.node` since it has no longer anything to do
with a "plugin".

Finally, by moving the `WorkflowNode`, `CalculationNode` and
`ProcessNode` classes in the `__init__` files of their respective
modules allows us to add further simplification to `load_node_class` as
well as making all these classes and their subclasses importable from
the `aiida.orm.node` module. Eventually, when `aiida.orm.data` is also
moved there, all classes can even be exposed directly on `aiida.orm` and
that is the deepest a user should ever go for they should need or be
allowed to import.
@sphuber sphuber force-pushed the fix_1603_node_class_loading branch from 5a26753 to 542a3b6 Compare January 18, 2019 17:52
@sphuber sphuber requested a review from giovannipizzi January 18, 2019 18:15
@sphuber
Copy link
Copy Markdown
Contributor Author

sphuber commented Jan 18, 2019

@giovannipizzi this is ready to go. You should check aiida.orm.utils.node. I have put back the support for loading external Data plugins, with the Data base class as a fallback. To test this I have added a test in aiida.backends.tests.orm.utils.node that checks that loading a node with an unknown type string will load the base data node class.

Copy link
Copy Markdown
Member

@giovannipizzi giovannipizzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Comment thread aiida/orm/utils/node.py
try:
base_path = type_string.rsplit('.', 2)[0]
except ValueError:
raise exceptions.MissingPluginError
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a next PR we should put a message to this exception

@giovannipizzi giovannipizzi merged commit be717b6 into aiidateam:provenance_redesign Jan 19, 2019
@sphuber sphuber deleted the fix_1603_node_class_loading branch January 19, 2019 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants