Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the %verdi IPython magics utility #5961

Merged
merged 2 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions aiida/tools/ipython/ipython_magics.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,57 @@ class AiiDALoaderMagics(magic.Magics):
@magic.needs_local_scope
@magic.line_magic
def verdi(self, line='', local_ns=None): # pylint: disable=no-self-use,unused-argument
"""Run the AiiDA command line tool, using the currently loaded configuration and profile."""
"""Run the AiiDA command line tool, using the currently loaded configuration and profile.

Invoking ``verdi`` normally through the command line follows a different code path, compared to calling it
directly from within an active Python interpreter. Some of those code paths we actually need here, and others
can actually cause problems:

* The ``VerdiCommandGroup`` group ensures that the context ``obj`` is set with the loaded ``Config`` and
``Profile``, but this is not done in this manual call, so we have to built it ourselves and pass it in with
the ``obj`` keyword.
* We cannot call the ``aiida.cmdline.commands.cmd_verdi.verdi`` command directly, as that will invoke the
``aiida.cmdline.parameters.types.profile.ProfileParamType`` parameter used for the ``-p/--profile`` option
that is defined on the ``verdi`` command. This will attempt to load the default profile, but here a profile
has actually already been loaded. In principle, reloading a profile should not be a problem, but this magic
is often used in demo notebooks where a temporary profile was created and loaded, which is not properly added
to the config file (since it is temporary) and so loading the profile would fail. The solution is to not call
the top-level ``verdi`` command, but the subcommand, which is the first parameter in the command line
arguments defined by the ``line`` argument.

"""
from click import Context

from aiida.cmdline.commands.cmd_verdi import verdi
from aiida.common import AttributeDict
from aiida.manage import get_config, get_profile

config = get_config()
profile = get_profile()
obj = AttributeDict({'config': config, 'profile': profile})
return verdi(shlex.split(line), prog_name='%verdi', obj=obj, standalone_mode=False) # pylint: disable=too-many-function-args,unexpected-keyword-arg

# The ``line`` should be of the form ``process list -a -p1``, i.e., a ``verdi`` subcommand with some optional
# parameters. Validate that the first entry is indeed a subcommand and not the flag to specify the profile which
# is not supported in this magic method.
cmdline_arguments = shlex.split(line)
command_name = cmdline_arguments[0]

if command_name in ('-p', '--profile'):
raise ValueError(
'The `-p/--profile` option is not supported for the `%verdi` magic operator. It will use the currently '
f'loaded profile `{profile}`'
)

# Construct the subcommand that will be executed, thereby circumventing the profile option of ``verdi`` itself.
# If the caller specified a subcommand that doesn't exist, the following will raise an exception.
context = Context(verdi)
Copy link
Member

Choose a reason for hiding this comment

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

Can you give me an example of what this context will be. Is this the profile name passed to the verdi command when -p is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right to point this out, if the user actually calls %verdi -p some-profile node show that will fail. But even if we were to parse it correctly, it would still fail if the specified profile is other than the currently loaded one. I think this behavior should not be supported. The only reason for the %verdi magic, as I understand it, is to be able to use a verdi command for the currently loaded profile in a notebook.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Then I think we can raise an exception explicitly if the user pass -p.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to be careful and also check for --profile. But yeah, I can add that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a check and raise if the profile parameter is specified.

command = verdi.get_command(context, command_name)

return command( # pylint: disable=too-many-function-args,unexpected-keyword-arg
cmdline_arguments[1:],
prog_name='%verdi',
obj=AttributeDict({'config': config, 'profile': profile}),
standalone_mode=False
)

@magic.needs_local_scope
@magic.line_magic
Expand Down
4 changes: 4 additions & 0 deletions docs/source/intro/tutorial.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ This tutorial can be downloaded and run as a Jupyter Notebook: {nb-download}`tut
:tags: ["hide-cell"]

from aiida import load_profile, engine, orm, plugins
from aiida.manage.configuration import get_config
from aiida.storage.sqlite_temp import SqliteTempBackend

%load_ext aiida
Expand All @@ -58,6 +59,9 @@ profile = load_profile(
),
allow_switch=True
)
config = get_config()
config.add_profile(profile)
config.set_default_profile(profile.name)
profile
```

Expand Down