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

initialization problem in fields #33

Open
denisri opened this issue Jul 26, 2023 · 3 comments
Open

initialization problem in fields #33

denisri opened this issue Jul 26, 2023 · 3 comments
Labels
6.0 bug Something isn't working

Comments

@denisri
Copy link
Contributor

denisri commented Jul 26, 2023

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.

@denisri denisri added bug Something isn't working 6.0 labels Jul 26, 2023
denisri added a commit to brainvisa/morpho-deepsulci that referenced this issue Jul 27, 2023
in order to avoid an initialization problem (populse/soma-base#33)
@sapetnioc
Copy link
Contributor

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
Copy link
Contributor Author

denisri commented Sep 19, 2023

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

@sapetnioc
Copy link
Contributor

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

sapetnioc added a commit that referenced this issue Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.0 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants