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

VIP1: Reworking functionality.yaml #1

Closed
8 tasks done
rcannood opened this issue May 1, 2020 · 7 comments
Closed
8 tasks done

VIP1: Reworking functionality.yaml #1

rcannood opened this issue May 1, 2020 · 7 comments

Comments

@rcannood
Copy link
Member

rcannood commented May 1, 2020

Viash Improvement Protocol 1

Some naming conventions in the functionality.yaml are, by now, somewhat illogical.

  • Issue A: resources typically have the same name as their path, so that could be dropped by default
  • Issue B: The platform variable is confusing with respect to the platform yaml. I would make this a variable in each of the resources instead, see example below.
  • Issue C: ftype should be renamed to function_type since no other names are abbreviated.
  • Issue D: I would like to define more default values. These changes will be reflected in the Scala code as well.
Example of current functionality.yaml
name: starReport
description: Generate STAR Report
platform: bash
ftype: convert
arguments:
- name: "--workDir"
  type: file
  required: true
  default: /tmp/STARR
  direction: input
- name: "--logFile"
  type: string
  required: true
  default: output.log
  direction: output
resources:
- name: main
  path: ./code.sh
- name: STARR.R
  path: ./STARR.R
- name: 01_featureAnnotation.R
  path: ./01_featureAnnotation.R
- name: 02_eSet.R
  path: ./02_eSet.R
Proposed format
name: starReport
description: Generate STAR Report
function_type: convert             # C) rename
arguments:
- name: "--workDir"
  type: file
  required: true
  default: /tmp/STARR
  direction: input                 # D) default, so can be dropped
- name: "--logFile"
  type: string                     # D) default??
  required: false                  # D) required: false is default
  default: output.log
  direction: output
files:
- type: bash_script                # B) change 'platform' so it is tightly coupled with the file it affects
  path: ./code.sh         
- type: resource                   # B) resource is the default type so can be omitted
  path: ./STARR.R
- path: ./01_featureAnnotation.R   # A) by default, the same name as specified in the path will be used
- name: foo.R                      # A) but this can be overridden
  path: ./02_eSet.R
- name: myfile.txt
  content: custom content          # B) you can still create files inside the yaml

Checklist:

If this VIP is accepted, update the following files

@rcannood
Copy link
Member Author

rcannood commented May 1, 2020

@tverbeiren What are the possible values in function_type?

@tverbeiren
Copy link
Member

tverbeiren commented May 1, 2020

Cfr B: Do I understand correctly that this allows us to perform Viash header/argparse parsing on this level as well? That would make a lot of sense IMO.

Cfr C: The current types I use are: asis, transform, convert, todir, join. However, these are neither consistent nor well thought out at this point. I should really take the time to take a step back (and decouple it more from the current NXF Target implementation).

@tverbeiren
Copy link
Member

tverbeiren commented May 1, 2020

@rcannood Why not specify (if relevant/appropriate) the type of the STARR.R script as well?

files:
- type: bash_script
  path: ./code.sh         
- type: r_script
  path: ./STARR.R

We could then add binary as well. Default could be text or something?

@rcannood
Copy link
Member Author

rcannood commented May 2, 2020

@rcannood Why not specify (if relevant/appropriate) the type of the STARR.R script as well?

I would use type: bash_script to denote that the script will need to be modified with a CLI using the functionality.yml, as well as know which script to run. As such, I would treat the STARR.R script as a text file, rather than an r_script. Does this seem fair?

The distinction between text and binary could be interesting, through I don't know when or where it would be used.

@rcannood
Copy link
Member Author

rcannood commented May 2, 2020

Cfr B: Do I understand correctly that this allows us to perform Viash header/argparse parsing on this level as well? That would make a lot of sense IMO.

What do you mean?

@tverbeiren
Copy link
Member

It would probably become too complicated at this point. Let's focus first on core functionality.

rcannood added a commit that referenced this issue May 6, 2020
@rcannood rcannood mentioned this issue May 6, 2020
@rcannood
Copy link
Member Author

rcannood commented May 6, 2020

I may have misinterpreted your last message, as I created a PR implementing these changes.

rcannood added a commit that referenced this issue Oct 6, 2020
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

2 participants