CLI: `verdi plugin list` fails for data plugins

[t-reents]

@edan-bainglass encountered the following issue and I've quickly investigated it.

Running e.g. verdi plugin list aiida.data core.array causes an exception:

verdi plugin list aiida.data core.array
Traceback (most recent call last):
  File "/Users/treents/.aiida_venvs/dev_aiida_core/bin/verdi", line 8, in <module>
    sys.exit(verdi())
             ^^^^^^^
  File "/Users/treents/.aiida_venvs/dev_aiida_core/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/treents/.aiida_venvs/dev_aiida_core/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/Users/treents/.aiida_venvs/dev_aiida_core/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/treents/.aiida_venvs/dev_aiida_core/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/treents/.aiida_venvs/dev_aiida_core/lib/python3.11/site-packages/aiida/cmdline/groups/verdi.py", line 117, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/treents/.aiida_venvs/dev_aiida_core/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/treents/.aiida_venvs/dev_aiida_core/lib/python3.11/site-packages/aiida/cmdline/commands/cmd_plugin.py", line 56, in plugin_list
    echo.echo(str(plugin.get_description()))
                  ^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Node.get_description() missing 1 required positional argument: 'self'

This error was introduced in #6411. In previous versions, e.g. aiida-core 2.5, the command would simply print the error: Error: No description available for core.array

Due to the changes in the aforementioned PR #6411, the try except is not triggered. As no AttributeError is raised anymore after adding the hasattr(plugin, 'is_process_function'): https://github.com/aiidateam/aiida-core/blob/fb3686271fcdeb5506838a5a3069955546b05460/src/aiida/cmdline/commands/cmd_plugin.py#L50-L58

Since the hasattr simply returns False in case of the ArrayData, the else statement is executed. Here, the error shown above is raised, as plugin.get_description() tries to call get_description on the class and not on an object. However, get_description is not a class method in orm.Node, leading to the exception.

Without thinking too much about it, one possible fix would be to create an object, i.e. calling plugin().get_description() (this would work for methods and class methods). However, I'm not sure if this will cause additional problems. Furthermore, one could catch the TypeError in the except statement as well. Again, these were just some ideas that came to my mind. There might be more things to consider that could lead to different solutions.

Pinging @sphuber and @khsrali who have worked on the previous PR.

[sphuber]

Thanks for the analysis @t-reents . Coincidentally, I had just responded to @edan-bainglass comment on the related PR. I will copy the comment here:

It is in fact related to this PR. The PR fixed a bug where the conditional would always raise an AttributeError for non-process plugins and so would always skip and simply print that no description was available. After this fix, the code now actually goes to the else-clause and calls plugin.get_description, but this is also a bug that simply was always hidden by the first bug. The get_description method is an instance method and the plugin is a class, so that call fails. There is no get_description classmethod though that provides a description of the plugin. The best we can do I think is to simply display the docstring of the plugin

As to your suggestion:

Without thinking too much about it, one possible fix would be to create an object, i.e. calling plugin().get_description() (this would work for methods and class methods).

This would be vulnerable still, because it requires constructing the class and that might require arguments. An example would be a Parser plugin which raises if you construct it without arguments.

What we really need to have is something that would work for all "plugins". And plugins here really can be any class, or even function really. So I think our best bet would be to use the docstring. It is guaranteed to exist and at worst returns None when not defined. We can check for this case and just print the default message that no help is available.

[t-reents]

Ah, I missed that Edan has already commented, I thought he stopped after our discussion :D sorry for the kind of duplicated issue

True, the Parser is one of the examples that I was assuming without having a specific example, thanks for the clarification.

The doc-string approach makes sense!

[khsrali]

Thanks @t-reents and @edan-bainglass for finding this issue. For some reason, I always assumed get_description() was defined as a class method across the interface. Now, by doing a simple search, just realized that's not always the case.

So for backward compatibility, I would keep the if.. else clause, and in case get_description() exists and is not a classmethod, handle the TypeError, and as @sphuber suggests print the docstring.