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

parameter and step groups #35

Open
tetron opened this issue Jul 17, 2024 · 11 comments
Open

parameter and step groups #35

tetron opened this issue Jul 17, 2024 · 11 comments

Comments

@tetron
Copy link
Member

tetron commented Jul 17, 2024

Want to be able to apply annotations to groups of parameters or workflow steps. This would principally be metadata documentation -- need to decide if this should include features that affect runtime execution or not. The goal is to have a way to provide better documentation & user interface for tool use.

Syntax options:

inputs:
  parameter1: string
  parameter2: int
groups:
  group1:
    groupMembers: [parmeter1, parameter2]
    doc: "parameters related to the foobar feature"
inputs:
  parameter1:
    type: string
    groupMember: group1
  parameter2:
    type: int
    groupMember: group1
groups:
  group1:
    doc: "parameters related to the foobar feature"

Discussion points

  • should groups include things that are meaningful to runtime, e.g. applying a hint or requirement to several steps? rejected
  • Can an item be a member of more than one group? rejected
  • Should it be possible for groups to be members of other groups? rejected
  • What kind of metadata should be specified?
    • label, doc, 'expanded'
  • Are groups the appropriate place to declare dependencies between inputs, for example if optional input A is specified then optional input B is actually required
    • The user guide describes option groups using records and unions but nobody uses that in practice
    • rejected
  • specifying order that parameters / groups appear in UI
    • suck for the user but decided we need this because we can't guarantee order in JSON/YAML documents
    • should decide if reusing "position" field is appropriate, in CommandLineTool it allows either integers or expressions, but expressions don't make sense here.
@tetron
Copy link
Member Author

tetron commented Jul 17, 2024

Groups should have a field called "expanded" (default true) which indicates if the group is expanded in the UI by default.

Groups should be displayed after any non-grouped items.

@t0mdavid-m
Copy link

I think this is an important issue. I agree that groups are necessary for UI applications. My personal preference would be the first syntax suggestion. I believe the second option could be a bit confusing to work with.

@mr-c
Copy link
Member

mr-c commented Nov 29, 2024

Here is an example of option one as an extension

$namespaces:
  cwltool: "http://commonwl.org/cwltool#"

inputs:
  parameter1: string
  parameter2: int

hints:
  cwltool:Groups:
    groups:
      group1:
        groupMembers: [parmeter1, parameter2]
        doc: "parameters related to the foobar feature"

Example of loading this using cwl-utils

from pathlib import Path

from cwl_utils.parser import load_document_by_uri
from cwl_utils.utils import normalize_to_map

cwl_file = Path("echo-groups.cwl")
cwl_obj = load_document_by_uri(cwl_file)
if cwl_obj.hints:
  hints = normalize_to_map(cwl_obj.hints, key_field="class")
  if 'cwltool:Groups' in hints:
     print(hints['cwltool:Groups'])

# {'groups': {'my_groups': {'groupMembers': ['first', 'second'], 'label': 'my great inputs', 'doc': 'parameters related to the foobar feature'}}}

@mr-c
Copy link
Member

mr-c commented Nov 29, 2024

@t0mdavid-m I updated the example in #35 (comment) to use the new syntax

@mr-c
Copy link
Member

mr-c commented Dec 2, 2024

Open question from a discussion with @mvdbeek

is groups mixed, for groups of inputs, groups of outputs, and (in case of a class: Workflow) groups of steps?
(with either a tricky schema enhancement or higher-level code to enforce that the groupMembers are all IDs from the same type)

or are there 3 top level entries, inputGroups, outputGroups, and stepGroups?

@mr-c
Copy link
Member

mr-c commented Dec 3, 2024

New idea for the native (non-extension) syntax: specifying directly as part of the inputs or outputs using a type: group fake input/output.

inputs:
  first_input: int
  advanced_params:
     type: group
     doc: "Advanced parameters related to the foobar feature"
     members:
       parameter1: string
       parameter2: int
  other_input: File

Using a group does not namespace the group members, their ids are parameter1 not advanced_params.parameter1; likewise the group doesn't exist in the data model; $(inputs.advance_params) has no meaning in the above example.

@tetron
Copy link
Member Author

tetron commented Dec 3, 2024

New idea for the native (non-extension) syntax: specifying directly as part of the inputs or outputs using a type: group fake input/output.

👎 this seems very awkward.

or are there 3 top level entries, inputGroups, outputGroups, and stepGroups?

I think we do want to this, but for a different reason: for schema-salad to generically validate cross-references, we need to at least distinguish between steps and parameters, and if we fix the naming collision between inputs/outputs in the future (by prefixing the ids that get assigned) then we'll also need separate inputGroups and outputGroups.

Revised proposal:

inputs:
  parameter1: string
  parameter2: int
inputGroups:
  - group1:
      groupMembers: [parmeter1, parameter2]
      expanded: true
      doc: "parameters related to the foobar feature"

inputGroups and groupMembers are both explicitly ordered. See #62 (comment) for discussion about ordered maps in YAML.

@mr-c
Copy link
Member

mr-c commented Dec 9, 2024

inputGroups and groupMembers are both explicitly ordered. See #62 (comment) for discussion about ordered maps in YAML.

Yes, that is good, but there is a (reasonable, in my opinion) user request to preserve the order of inputs or outputs between the YAML file and any UI interface that is made using them. Regardless if those inputs/outputs are in a group or not.

@mr-c
Copy link
Member

mr-c commented Dec 9, 2024

New idea for the native (non-extension) syntax: specifying directly as part of the inputs or outputs using a type: group fake input/output.

👎 this seems very awkward.

Why? Because of the fake type?

@tetron
Copy link
Member Author

tetron commented Dec 9, 2024

Why? Because of the fake type?

I think I misunderstood the proposal. What you're saying is that the input parameters are first_input, parameter1, parameter2, and other_input, e.g. the input object would still be

first_input: 5
parameter1: "hello"
parameter2: 10
other_input:
  class: File
  location: http://example.com/file.jpg

Using the proposed sequence syntax and adding in label and expanded here's what it would look like:

inputs:
- first_input: int
- advanced_params:
     type: group
     label: "Advanced parameters"
     doc: "Advanced parameters related to the foobar feature"
     expanded: false
     members:
     - parameter1: string
     - parameter2: int
- other_input: File

This does feel a bit easier to understand for the end user. On the other hand, embedding groups into the input definition like this means it is no longer an external annotation. This means both the validation code and any code that walks over the list of inputs has to be able to handle groups. So it is a much bigger change for implementers. (which is why the original proposal defined groups separately).

@mr-c
Copy link
Member

mr-c commented Dec 12, 2024

This means both the validation code and any code that walks over the list of inputs has to be able to handle groups. So it is a much bigger change for implementers. (which is why the original proposal defined groups separately).

With this fake "type: group", will we need to change anything in Schema Salad to ensure a single namespace for all the inputs, regardless of their group?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants