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

Add Wordcloud #76

Merged
merged 6 commits into from
Oct 16, 2024
Merged

Add Wordcloud #76

merged 6 commits into from
Oct 16, 2024

Conversation

524D
Copy link
Contributor

@524D 524D commented Oct 14, 2024

Added wcloud.

I still need to merge changes that were made in the meantime, but can you please check if things look okay?

@vedran-kasalica
Copy link
Member

vedran-kasalica commented Oct 14, 2024

Hi @524D, thank you for making the PR the changes look good! I see that the tool.json file is missing cwl_implementation field (see documentation, specifically text above "Adding New Tools from bio.tools Not Present in cwl-tools").

The newest APE-2.4 version dev3 generates cwl_implementation field as well (it just has to be updated to point to GitHub).

Finally, it would also be nice at some point to include the Dockerfile under cwl-tools/wcloud/docker, similar was done for MSAmana. I shared with you access to the Workflomics account on DockerHub via LastPass, in case you want to upload the images under Workflomcis.

@524D
Copy link
Contributor Author

524D commented Oct 15, 2024

Hi @vedran-kasalica, thanks for your remarks. I've updated the files and merged changes that were done in the meantime. According to GitHub it can now be merged, but I still have done insufficient testing, especially of the CWL. Is an easy way to do this?

I will include the Dockerfile at a later point, it's now in a separate repository.

Edit: I've tested with cwl-runner by creating a wordcloud from a license file. It works as expected, so I hope perform correctly in the workflomics benchmarks.

@kretep
Copy link
Contributor

kretep commented Oct 15, 2024

@524D If you want, you could add a test script similar to that in Sage-proteomics: https://github.com/Workflomics/tools-and-domains/tree/main/cwl-tools/Sage-proteomics/test

If you name the script "run-cwl.sh", it will be included in the automated tests. You'll need to put your data somewhere, or indeed just link to a random text file and have "input.yml" refer to that.

Copy link
Member

@vedran-kasalica vedran-kasalica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Rob, thank you for updating the PR, the tool looks ready to be used within Workflomics! Feel free to merge it into main.

Regarding your question about testing, Peter started implementing automated tests by adding sh scripts to tool dirs see Sage. Here, run_cwl.sh annotates the command to run the cwl using cwltool and the predefined inputs.yml. If you wish you can add a similar file to your repo to trigger the automated test, but this could be done in a separate PR.

@524D 524D merged commit 06ee052 into Workflomics:main Oct 16, 2024
1 check passed
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.

3 participants