-
Notifications
You must be signed in to change notification settings - Fork 14
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
Feature/simplify CTS usage #230
base: develop
Are you sure you want to change the base?
Conversation
If this is consumed by a single tool later on, I would just add this logic on that tool, otherwise it is an extra box just to write params in YAML. |
This was my idea, we need to provide config to more than one downstream tool I think. |
But yes @a-solovyev12 if I've mis-understood and the config is only passed to a single tool, then that tool should have these options instead. |
@pinin4fjords This config is passed to two scripts: a general data import and classifier import ones. Only the latter is relevant in the context of Galaxy, that is why I said it works for a single tool. Putting all of these parameters into that import script will make the options cluttered and basically defeat the purpose of this config file, so in my opinion it's better to leave it as an independent tool. |
Re options cluttered, you can always hide advanced options. Is it worth it to send a job to the cluster just to create a YAML that will be used once and where there is no flow control involved for the workflow (as in for instance what we do for iterators?)? |
I agree, if the options only pertain to one (or even two) tools then they belong on that tool rather than a separate config step. This is partly down to my confusion, so apologies for that. |
Description
Type of change
Checklist
@TOOL_VERSION@
), then I have reset all 'build' values to 0 (e.g.@TOOL_VERSION@+galaxy0
)@TOOL_VERSION@+galaxy0
@TOOL_VERSION@+galaxy1
)