Enum items property is incorrect when a validate method is provided

[frmdstryr]

class Foo(Atom):
    MAPPING = {"x": 1, "y": 2}
    x = Enum(*MAPPING.keys())
    

Foo.x.items
Out[21]: ('x', 'y')

class Foo(Atom):
    MAPPING = {"x": 1, "y": 2}
    x = Enum(*MAPPING.keys())
    def _validate_x(self, old, new):
        return new
        

Foo.x.items
Out[23]: '_validate_x'

Edit. From looking at the code I suppose this could just be considered incorrect usage. I'm trying to track which fields are invalid. Maybe there is a better way to do it? I just decided to subclass the Enum and override the items property.

class Foo(Atom):
    MAPPING = {"x": 1, "y": 2}
    errors = Dict()
    x = Enum(*MAPPING.keys())
    def _validate_x(self, old, new):
        if new not in self.MAPPING:
            self.errors['x'] = "x is invalid"
            return old     
        return new

[MatthieuDartiailh]

So you are definitively in the gray region of what is valid when customizing members. Enum is not the only that can be broken in this way I believe.

The central issue here is that the items are only stored in the validate_mode so if you customize it there is no way to recover items. It won't fix your issue but we could have items raise if the validate mode is not Enum. At least it would avoid ending with a broken beahvior silently. WDYT ?

[frmdstryr]

have items raise if the validate mode is not Enum

I think that'll work, have the error suggest overriding it as I did.

[frmdstryr]

What would you think about adding a pre_setattr that would run before validate?

Another thing I ran into recently is trying to use a custom list (MutableMapping) with a List field. If I use coerced then it does not validate the list items.

[MatthieuDartiailh]

If you want an homogeneous user experience it would have to be a pre_validate (since otherwise a default value would be treated differently from a set value). I can see the interest in that it would allow to decouple the typing validation from type coercion which is a desirable feature I believe but I am not yet sure it is the right way to go about it.

Could you expand on your use case ?

[frmdstryr]

I am trying to have a list member with custom methods to commit changes to the database. Basically the same as django's related manager. See code and usage.

It uses post_getattr to convert to a RelatedList which works, but the problem is that it breaks assignment unless I cast it back with list(). With a pre_validate I could convert it back to a normal list while still having the list items be validated by atom.

[MatthieuDartiailh]

Out of cusiority have you considered directly coercing to your custom type and make it wrap an atom list ? And use post_setattr to manage observers ?

[MatthieuDartiailh]

For your particular case I feel like you could use a Coerced and some post_setattr customization to handle observers.

That being said I was always a bit bothered by the way coercion and type validation worked (or rather did not work together). So if you feel up to implementing a pre_validate mechanism please go for it ! The addition of the getstate mode mean you have room to store the mode on the member.

[MatthieuDartiailh]

And sorry for the late reply

[MatthieuDartiailh]

I actually gave some extra though about having coercion as a pre-validate mode and it does not really work out that well. Typically one wants coercion to occur if the item fails to validate so it cannot run before validation.

One thing we could however consider is to introduce a coerce mode and to modify the generic validation path to attempt coercion if validation fails on the input. Such a solution would viable I believe and reduce the coupling between type validation and coercion. WDYT ?