👌 IMPROVE: Hide dev/non-public API methods

[mbercx]

Is your feature request related to a problem? Please describe

When using tab-completion in an IPython kernel (console or notebook), the user gets a very long list of methods which are largely not really part of what one might consider to be the public API:

This can be overwhelming for new users, and makes it very hard to explore the features of a Node class without reading the documentation.

Describe the solution you'd like

While he was working on #4975, @chrisjsewell explained to me that the __dir__() method can be used to override the list of methods that are shown when using tab-completion. We then considered that this might also be used to achieve a cleaner list of methods for the user for already existing Node classes.

Developing in an IDE with intellisense shouldn't be affected, and we could consider writing some IPython magic to reactivate tab-completion for all methods, or even make this configurable with verdi config.

Additional context

This issue was also raised by @louisponet during his code show-and-tell, as well as by other users during tutorials etc.

[chrisjsewell]

So I just want to expand on this a bit:

The number of attributes/methods on Node is way too high, and this is exacerbated further on subclasses, that have their own attributes/methods:

In [1]: node_props = {n for n in dir(Node) if not n.startswith('_')}
In [2]: len(node_props)
Out[2]: 84
In [3]: dict_props = {n for n in dir(Dict) if not n.startswith('_')}
In [4]: len(dict_props)
Out[4]: 99

Not only that, but they are also not particularly helpfully named for tab completion, e.g. you could argue that get_attribute/set_attribute, would be easier as attribute_get/attribute_set.

One problem here is that, in general programming, you will have two types of methods; private and public.
For nodes though, we essentially want three:

1. Ones that developers of aiida-core use
2. Ones that developers of a data plugin use
3. Ones that users of a data plugin use

As a pseudo example

# aiida-core developers

class Entity:
    def __init__(self, backend_entity):
        self._backend_entity = backend_entity
    @property
    def backend_entity(self):
        return self._backend_entity

class Node(Entity):
    def set_attribute(self, key: str, value: Any):
        if self.is_stored:
            raise exceptions.ModificationNotAllowed('the attributes of a stored entity are immutable')
        self.backend_entity.set_attribute(key, value)

# plugin developers

class Dict(Node):
    def __setitem__(self, key, value):
        self.set_attribute(key, value)

# users

dict = Dict()
dict["key"]

In this example:

• Plugin developers should never access Node.backend_entity
• Users should never access Dict.set_attribute

We could override the __dir__() method, but I think this is somewhat a band-aid and not a solution.

One specific problem I see, is the use of mixins:

class Node(Entity, NodeRepositoryMixin, EntityAttributesMixin, EntityExtrasMixin):

IMO, mixins are never usually a good solution, and one should always prefer composition over mixins (see https://en.wikipedia.org/wiki/Composition_over_inheritance), e.g.

class Node(Entity):
    def __init__(self):
        self.repo = NodeRepository(self)
        self.attributes = EntityAttributes(self)
        self.extras = EntityExtras(self)

One would then use e.g. node.attributes.set(), rather than node.set_attributes(), which

1. Makes tab-completion a lot more user-friendly, by grouping methods/attributes under namespaces
2. Implementing just this would already reduce the Node method/attributes count to 50 (i.e. reducing by 34)

(note this is adapted #5088 (comment))

[chrisjsewell]

Phase 1 😉 #5439

[chrisjsewell]

Phase 2 #5442

[chrisjsewell]

Phase 3 #5445

[chrisjsewell]

Phase 4 #5446 (and #5447)

Down to 43 public attributes/methods on Node

['Collection', 'add_incoming', 'attrs', 'backend', 'backend_entity', 'class_node_type', 'clear_hash', 'comments', 'computer', 'ctime', 'description', 'get', 'get_all_same_nodes', 'get_cache_source', 'get_description', 'get_hash', 'get_incoming', 'get_outgoing', 'get_stored_link_triples', 'has_cached_links', 'id', 'initialize', 'is_created_from_cache', 'is_stored', 'is_valid_cache', 'label', 'logger', 'mtime', 'node_type', 'objects', 'pk', 'process_type', 'rehash', 'repo', 'store', 'store_all', 'user', 'uuid', 'validate_incoming', 'validate_outgoing', 'validate_storability', 'verify_are_parents_stored', 'xtras']

[sphuber]

Since we have had significant improvements, I will close this for now. If there are still concrete proposals for methods to be moved, a new issue can be opened