-
Notifications
You must be signed in to change notification settings - Fork 17
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
Consider adding a context
to the Port
validation methods
#126
Comments
@muhrin happy to have your input. I think technically it would be feasible but I am not sure if this would start to couple nested namespaces to deeply to the namespace in which they are nested. I know that the |
Yup. In general I like the 'context concept'. I think for this kind of validation it's perfectly reasonable for validity to be a function of your siblings/parents/etc, even though we have to acknowledge that this allows for a greater coupling between validators and therefore less reusability. But I think this is a price worth paying. If you're up for it I can put together a test implementation and we can see if It would meet your use case. |
Thanks but no need, I already have a starting implementation that seems to work. I will create a PR soon so we can discuss/iterate. |
Top lad. Looking forward to it. If possible I would vote for a non-breaking implementation, say keyword argument with default for the context but I'm happy give my thoughts on whatever you bake up. |
In any case it will be breaking existing validators I am afraid, because now we will be passing a second argument at a minimum, but current validators will only allow one in their signature. Unless we start inspecting the argspec of validators before calling them, emitting a deprecation warning if we detect that they only define a single argument. That might be necessary thinking about it. |
This use case stems from the desire of writing a port
validator
whose implementation does not only require the input values for the port that it is set for, but also other inputs that are passed elsewhere in the namespace (even higher up the nesting). I am not sure if this is an indication of a badly designed input namespace, but it seems reasonable. Example, imagine the following namespace:the validator
validate_a
with current implementation will only receive the input passed fornamespace.a
. But if its validation depends on the value ofnamespace.b
it is impossible. One solution may be, simply moving the validator to the top level namespace:However, this may not always be possible. The actual use case stems from
aiida-core
where theCalcJob
currently puts the validation of themetadata.options.resources
on the top level namespace, but this validator includes other validation as well. In a sub class I know want to make theresources
optional and so need to remove the validation of the resources in the top level validator and want to move it directly to themetadata.options.resources
port. This would allow me to override that in the sub class, affecting only the part that I want to target. However, this is currently not possible, since the validation of resources depends on thecomputer
input which is passed either indirectly through thecode
input or directly in themetadata.computer
port. In either case, the validator will only get the values passed formetadata.options.resources
directly and it is impossible to access the computer.The text was updated successfully, but these errors were encountered: