populse / soma-base

Miscelaneous all-purpose classes and functions
0 stars 3 forks source link

initialization problem in fields #33

Open denisri opened 1 year ago

denisri commented 1 year ago

In pydantic Controller, we can declare a class field this way:

class SulciLabling(Controller):
    labeled_graph: File = field(write=True, doc='output labeled graph')

However the field() function, in this case, is called without a type_ argument, and metadata are not initialized properly:

>>> p = SulciLabeling()
>>> print(p.field('labeled_graph')).metadata()
{'write': True,
 'doc': 'output labeled graph',
 'order': 1000077,
 'path_type': None,
 'class_field': True}

see: path_type is None, not 'file' If we use:

class SulciLabling(Controller):
    def __init__(self):
        super().__init__()
        self.add_field('labeled_graph', File, write=True, doc='output labeled graph')

then the metadata are:

{'write': True,
 'doc': 'output labeled graph',
 'order': 1000078,
 'path_type': 'file',
 'read': True,
 'class_field': False}

A solution is to double-specify the type in the class field declaration:

class SulciLabling(Controller):
    labeled_graph: File = field(type_=File, write=True, doc='output labeled graph')

but it needs to say things twice, is error prone, and it was not designed to be this way. I don't know (or don't remember) how class fields are assigned in the 1st form: field() returns a Field instance without a type set, then it seems to be modified when assigned to the typed class variable labeled_graph, but I don't know where and if we can do anything there to fix things ?

The consequence of this bug is that, in capsul, tests done in fields like: if field.path_type are sometimes wrong and unreliable.

sapetnioc commented 1 year ago

I already noticed (and forgot) problems using this syntax. I now believe that the syntax is wrong since the affectation should contain a value not a type. The right syntax should be:

class SulciLabling(Controller):
    labeled_graph: field(type_=File, write=True, doc='output labeled graph')

I am in favor of raising an error when the value set on a controller attribute is a Field. What do you think ?

denisri commented 1 year ago

I agree to raise an error when doing wrong things. Do you (we) have control over this class attribute assignment to check it ?

sapetnioc commented 1 year ago

In Controller subclasses yes, via a Metaclass that parse the class definition.