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

Added expression tool to ease parameter sweeps of recetox-aplcms on Galaxy #624

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hechth
Copy link
Member

@hechth hechth commented Dec 4, 2024

This PR adds an expression tool to the recetox_aplcms suite that parses the parameters sweeped during peak detection.
This tool can be extended to contain more parameters in the future.

@hechth hechth requested a review from bgruening December 4, 2024 14:26
@hechth hechth marked this pull request as ready for review December 4, 2024 14:35
@hechth hechth linked an issue Dec 4, 2024 that may be closed by this pull request
@hechth
Copy link
Member Author

hechth commented Dec 4, 2024

maybe @mvdbeek has an opinion on this as well? I didn't figure how to read the passed csv file in the ECMA script but I tried to include some validation logic on the file contents.

@mvdbeek
Copy link

mvdbeek commented Dec 4, 2024

how to read the passed csv file in the ECMA script b

that's the point of it, you can't do I/O while executing ecmascript making this a safe choice for evaluating expressions. load_contents="64000" means your loading up max 64000 characters from your file.

@hechth
Copy link
Member Author

hechth commented Dec 4, 2024

how to read the passed csv file in the ECMA script b

that's the point of it, you can't do I/O while executing ecmascript making this a safe choice for evaluating expressions. load_contents="64000" means your loading up max 64000 characters from your file.

Thanks, ok that makes sense. So do you think that the tool is fine like this? Probably the number of characters can be significantly reduced but apart from that? @bgruening has some concerns about this

@mvdbeek
Copy link

mvdbeek commented Dec 4, 2024

Honestly this is brilliant and what I hope will happen more in the future. What are the concerns ?

@mvdbeek
Copy link

mvdbeek commented Dec 4, 2024

When I say more I hope we can just let people write these expressions directly without having to write a separate standalone tool. The UI will be tricky to get right, but if you only do the one exact thing you need to do I think this reduces workflow and tool complexity a lot.

@bgruening
Copy link
Collaborator

My concern was that it is too specific to the aplcms use-case. I would write this tool more generally and get it upstream or in IUC. I will look at it more tomorrow.

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

Successfully merging this pull request may close these issues.

recetox-aplcms: parse parameter line tool
3 participants