Remove attribute from PClass sets attribute to initial value

[jongiddy]

PClass is documented as behaving like PRecord (except that it is not a collection).

However, there is a difference in the behaviour when removing attributes.

Removing an attribute from a PRecord either deletes the attribute, or, if the field is mandatory, fails with an InvariantException.

class X(PRecord):
    a = field(initial=1)

x = X()
x = x.set(a=2)
x = x.remove('a')
assert x.get('a') is None

Performing the equivalent operations on a PClass returns the attribute to the initial value, whether the attribute is mandatory or not.

class X(PClass):
    a = field(initial=1)

x = X()
x = x.set(a=2)
x = x.remove('a')
assert x.a == 1

Changing the behaviour to be more consistent would be a major backwards-incompatible change, so the initial fix might update the docs for PClass to describe this additional difference.

[tobgu]

Thanks for finding this inconsistency. mandatory in combination with initial can give rise to situations where there is no obvious best behavior.

Perhaps the best long term solution would be to rename initial to default (keeping initial as a deprecated alias for some time of course). If doing so the current behavior of the PClass would probably make most sense. Setting a field that has a default value to mandatory would then be meaningless.

Don't know if you have any thoughts on this?

[jongiddy]

I like the idea to rename the current PClass behaviour as default, to distinguish it from the PRecord behaviour of initial.

Eventually, following deprecation and based on demand, PRecord and PClass could support both initial and default.

On initialization, the sequence would be as if:

1. set field to default value, if provided.
2. set field to initial value, if provided, overriding any previous value.
3. set field to parameter value, if provided, overriding any previous value.
4. if field is mandatory and unset, raise InvariantException.

The initial value is never re-used following initialization.

On remove, the sequence would be as if:

1. if field has default value, set field to default.
2. else if field is mandatory, raise InvariantException.
3. else remove field key/attribute.

[tobgu]

Yes, something like that but I think that one of initial and default should go away. Since there is not real distinction between a new object (that would trigger the initial value to be used in your example) and the evolution/mutation of an existing object (which would only make use of the default value) which also results in a new object it may be confusing to have different behaviors. Don't you think? Or do you have a good use case for when initial would actually be good to have in addition to default?

[jongiddy]

No, I don't have a use case. The current different behavior for PRecord's and PClass's seems sensible, since a Python class behaves the same way as a PClass. However, the use of the initial keyword for a value that can be seen again is surprising. So, I agree that moving to default for the PClass type only would make sense.