Add required flag to DIVE_PARAM#1639
Conversation
Superseded by the `required` flag on DIVE_PARAM (see PR #1639). VIAME \`.pipe\` files using the old `# Requirements:` header should be migrated to the new spec — until they are, those parameters won't be prompted for. - Drop \`PipelineRequirement\` from apispec.ts and dive_utils/types.py. - Drop \`description\` and \`requirements\` fields from \`Pipe\` / \`PipelineDescription\`; description is already exposed via \`metadata.description\`. - Drop \`extract_pipe_description\`, \`extract_pipe_requirements\`, \`extractPipeDescription\`, \`extractPipeRequirements\`. Training config descriptions now read from \`extractPipeMetadata().description\`. - Drop the Requirements dialog from RunPipelineMenu.
389b487 to
a10f41e
Compare
|
Hi, For example to pass from: And we would be able to combine it with the required argument: It is a bit more verbose, but I think it could be more scalable, add avoid problems if later we have a string parameter for a type. What do you think of this? |
Pipelines can now mark a DIVE_PARAM as required by adding `required`
as one of the type props in the comment header:
# DIVE_PARAM ["My label", float, required]
:my:key = 0.5
The parser strips it from `type_props` and sets `required: true` on
the param. PipelineParamsDialog shows a red `*` on the label and
blocks the Run button via vuetify form validation if the field is
empty. Both the desktop common.ts parser and the server
pipeline_discovery.py parser handle the keyword.
06e04b6 to
0feff06
Compare
| const rules: Record<string, ValidationRule> = { | ||
| integer: (value: string | number) => Number.isInteger(Number(value)) || 'Please enter a whole number', | ||
| positive: (value: string | number) => Number(value) >= 0 || 'Please enter a number ≥ 0', | ||
| strictlyPositive: (value: string | number) => Number(value) > 0 || 'Please enter a number > 0', | ||
| }; | ||
|
|
||
| function getRules(type: PipelineParamType): ValidationRule[] { | ||
| const res = []; | ||
| function getRules(type: PipelineParamType, required = false): ValidationRule[] { | ||
| const res: ValidationRule[] = []; | ||
| if (required) { | ||
| res.push((value) => { | ||
| if (value === undefined || value === null || value === '') return 'Required'; | ||
| return true; | ||
| }); | ||
| } | ||
| if (type.includes('int')) { | ||
| res.push(rules.integer); | ||
| } |
There was a problem hiding this comment.
I think we can integrate the rule directly in the rules dict to have it consistent.
| } | |
| const rules: Record<string, ValidationRule> = { | |
| integer: (value: string | number) => Number.isInteger(Number(value)) || 'Please enter a whole number', | |
| positive: (value: string | number) => Number(value) >= 0 || 'Please enter a number ≥ 0', | |
| strictlyPositive: (value: string | number) => Number(value) > 0 || 'Please enter a number > 0', | |
| required: (value: string | number) => (value !== undefined && value !== null && value !== '') || 'This field cannot be empty', | |
| }; | |
| function getRules(type: PipelineParamType, required = false): ValidationRule[] { | |
| const res: ValidationRule[] = []; | |
| if (required) { | |
| res.push(rules.required); | |
| } | |
| if (type.includes('int')) { | |
| res.push(rules.integer); | |
| } |
There was a problem hiding this comment.
Otherwise looks good to me
Summary
Pipelines can now mark a `DIVE_PARAM` as required so the user can't run the pipeline until they supply a value, and `config ` lines (kwiver globals) can be DIVE_PARAM targets — same syntax, parser-side recognition only.
Spec
```
:my:key = 0.5 # DIVE_PARAM ["My label", float, required]
config global:fps = 5 # DIVE_PARAM ["FPS", int, required]
```
`required` is a flag keyword in the DIVE_PARAM comma list — stripped from `type_props`, sets `required: true` on the param.
Coverage
Changes