Add possibility to remove inputs from a builder with delattr

[giovannipizzi]

Currently, once you have a builder, you might want to remove an optional output (e.g. if you get a builder from a get_builder_restart and want to drop an optional input).

Currently, this is achievable by deleting an item using dictionary syntax:

del builder['input_name']

However, the following syntax does not work:

del builder.input_name

I suggest to support also this syntax, since it is already possible (and 'suggested') to use builders with attribute access for setting data, thanks to the TAB-completion support (this can just proxy to the existing deletion function for dictionary items).

[ramirezfranciscof]

@sphuber does this require any discussion? In some way it feels similar to the recent changes in dict behaviour, but maybe there is a reason for the builder specifically not to allow this? If you can't think of one, I can start working on this.

[sphuber]

No, this should be extremely straightforward. I don't see any problems with this, just implement __delattr__ and add a test and it should be good to go

[greschd]

So there's one subtlety here, which I think we should be aware of but probably doesn't affect the decision on this:

When deleting an input with del builder['input_name'], the builder will still have the input_name attribute. See the following example:

In [34]: builder = PwCalculation.get_builder()

In [35]: builder.code = <some_code>

In [36]: del builder['code']

In [37]: builder.code # The builder has a 'code' attribute (actually, a property) that returns `None`

In [38]: del builder['code'] # Deleting a second time doesn't work
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
<ipython-input-38-bdc7a2e3ca23> in <module>
----> 1 del builder['code']

/data/aiida/source/aiida-core/aiida/engine/processes/builder.py in __delitem__(self, item)
    111
    112     def __delitem__(self, item):
--> 113         self._data.__delitem__(item)
    114
    115     def _update(self, *args, **kwds):

KeyError: 'code'

In [39]: hasattr(builder, 'code')
Out[39]: True

So when translating that to del builder.code, should the second deletion raise, or just silently do nothing? Since hasattr(builder, 'code') is True I'd expect the latter. It's slightly inconsistent that del builder.code still retains the code attribute - but it's probably ok to do so.

[ramirezfranciscof]

@greschd Nice catch. So, this is not the "correct" python behavior, right? I mean, when you del builder['input_name'], the key gets completely removed. Why is it different here?

[greschd]

So, this is not the "correct" python behavior, right?

I think it's kind of a grey area. The reason it's different here is that it's not a "normal" instance attribute, but a property.

There isn't really an explicit specification of what del should do to properties as far as I'm aware. A quick search in the standard library showed this, however:

In [2]: from urllib.request import Request

In [3]: r = Request(url='http://www.aiida.net/')

In [6]: r.full_url
Out[6]: 'http://www.aiida.net/'

In [7]: del r.full_url

In [8]: r.full_url

In [9]: del r.full_url

Going by that, it's fine to return None after the deletion - but a subsequent second deletion should just do nothing, instead of raising.

[sphuber]

I guess the problem with the example that you sketch is not that we actually want to be deleting the attribute, we are just setting its value to None. The attribute should still remain a property setter and getter, otherwise it won't be able to set a value after having deleted it. There is another subtlety that I am now thinking off. Given that passing an input with None is different from not passing the input at all, as far as AiiDA is concerned, maybe del should actually remove the key. I will try to sketch what I think the behavior should be:

builder = PwCalculation.get_builder()
builder.code                # this prints the current value
dict(builder)               # {}
builder.code = None         # Set it to `None`
dict(builder)               # {'code': None}
del builder.code            # Will delete the key from the internal mapping, but the property still exists
dict(builder)               # {}
buider.code = load_code(1)  # This should still work
dict(builder)               # {'code': Code<1>}

del builder.code
del builder.code  # Deleting twice should not raise

[greschd]

Given that passing an input with None is different from not passing the input at all, as far as AiiDA is concerned, maybe del should actually remove the key. I will try to sketch what I think the behavior should be:

Right, I think that makes sense. Would that imply that del builder.input_name should work only for optional inputs, and raise for required ones? Or not, because the builder itself is still in a valid (although incomplete) state?

[sphuber]

Given that passing an input with None is different from not passing the input at all, as far as AiiDA is concerned, maybe del should actually remove the key. I will try to sketch what I think the behavior should be:

Right, I think that makes sense. Would that imply that del builder.input_name should work only for optional inputs, and raise for required ones?

Don't think so, because you could just be setting it later before submitting, even though that might not necessarily make sense.
The following:

del builder.code
builder.code = load_code(1)

should be just fine I think

[greschd]

Yep, agreed. As I realized in my sneaky edit above, the builder state is valid even if the code is deleted. After all, a clean builder also doesn't have all required inputs.

I think one could come up with a scenario where this makes sense - e.g., you have one piece of code responsible for "cleaning up" the builder, and then another one re-populating it.