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

Removing nodes that aren't in Group raises no warning #4438

Closed
mbercx opened this issue Oct 12, 2020 · 1 comment · Fixed by #4728
Closed

Removing nodes that aren't in Group raises no warning #4438

mbercx opened this issue Oct 12, 2020 · 1 comment · Fixed by #4728

Comments

@mbercx
Copy link
Member

mbercx commented Oct 12, 2020

Currently, removing nodes that aren't in a Group raises no error or warning. E.g. in the verdi shell:

In [1]: from aiida import orm
In [2]: group = orm.Group('test')
In [3]: group.store()
Out[3]: <Group: "test" [type core], of user mbercx@gmail.com>
In [4]: node = orm.load_node(1)
In [5]: group.remove_nodes(node)

The last command raises no error or warning, giving the user the impression that the command has been successfully executed. This behavior is even more confusing when using the CLI, since a prompt is raised that asks if you want to remove from the group:

$ verdi group remove-nodes -G test 1
Do you really want to remove 1 nodes from Group<test>? [y/N]: y
$

Expected behavior

I would prefer to see a warning raised for each node that the user tries to remove from the group that is actually not in the group, similar to the linux rm command. Nodes that are in the group are still removed, however.

The behavior should be the same for the Python command Group.remove_nodes() and CLI command verdi group remove-nodes. So perhaps this should be implemented at the Group or BackendGroup level?

As a side note: if the user tries to remove nodes that do not exist in the database, an error is raised and no nodes are removed from the group. Perhaps the behavior here should be similar to the case where the node is not in the group?

Also see this discussion in #4428.

@giovannipizzi
Copy link
Member

I would be tempted to say: we leave the python API as is, and we just implement the information on the command line, something like:

$ verdi group remove-nodes -G test 1 2 3
[WARNING] Two nodes you specified (2, 3) are not in the group 
Do you really want to remove 1 nodes from Group<test>? [y/N]: y

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants