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 6 new tools to cpt.yaml #776

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

Conversation

jasonjgill
Copy link
Contributor

Adds 3 new CPT phage tools, deletes 2 deprecated CPT tools, adds 3 new useful phage tools from the toolshed.

Add 5 tools, remove 2 tools
added defense_finder
@@ -64,18 +64,10 @@ tools:
owner: cpt
tool_panel_section_label: Annotation

- name: cpt_intersect_adjacent
Copy link
Member

Choose a reason for hiding this comment

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

We can not remove tools, they need to stay for reproducibility ... we could hide them if this is useful.

Please let them listed here and add a comment that they are deprecated.

cpt.yaml Outdated
tool_panel_section_label: Annotation

- name: phageterm
owner: mmonot
Copy link
Member

Choose a reason for hiding this comment

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

We need to review those tools before we can merge them.

Copy link
Member

Choose a reason for hiding this comment

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

Can you point me maybe to the github repo of those tools?

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/avilella/phageterm <- is a fork

The only reference on github is https://github.com/TAMU-CPT/galaxy-tools/blob/c43f5a7706fb64615b7c050a5a9e5fc20ca33873/tool_conf.xml.old#L76 ... any reason you have hidden it on your instance?

I think I found the latest version https://gitlab.pasteur.fr/vlegrand/ptv/-/tree/master

But the Galaxy wrapper is 6 years old and has not been updated. I could not find the source on github, the source in the TS is insecure and needs updates.

@jasonjgill
Copy link
Contributor Author

jasonjgill commented Aug 30, 2024 via email

Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

@jasonjgill I tried to find the source of the tools, see my comments inline.

I'm not concerned about functionality. I trust you all when you say those tools are useful. My problem is with sustainability and security. When those wrappers are not maintained over time, do not have tests, or do not adhere to basic security needs of a public Galaxy server, I have problems to install them.

Thats why I recommend to contribute your tools to IUC as well. There we have a review process, rigorous testing and bots that help us maintaining the warppers.

cpt.yaml Outdated
tool_panel_section_label: Annotation

- name: phageterm
owner: mmonot
Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/avilella/phageterm <- is a fork

The only reference on github is https://github.com/TAMU-CPT/galaxy-tools/blob/c43f5a7706fb64615b7c050a5a9e5fc20ca33873/tool_conf.xml.old#L76 ... any reason you have hidden it on your instance?

I think I found the latest version https://gitlab.pasteur.fr/vlegrand/ptv/-/tree/master

But the Galaxy wrapper is 6 years old and has not been updated. I could not find the source on github, the source in the TS is insecure and needs updates.

cpt.yaml Outdated

- name: phagetermvirome
owner: bioit_sciensano
tool_panel_section_label: Annotation
Copy link
Member

Choose a reason for hiding this comment

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

This one also seems to be https://gitlab.pasteur.fr/vlegrand/ptv/-/tree/master

The previous PhageTerm program (single phage analysis only) is still available at https://sourceforge.net/projects/phageterm/ (for versions <3.0.0)

But also no Galaxy wrapper in github/gitlab.

The wrapper in the TS looks ok, a few things can be improved.

cpt.yaml Outdated
tool_panel_section_label: Annotation

- name: defense_finder
owner: rplanel
Copy link
Member

Choose a reason for hiding this comment

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

That one looks ok, I found the wrapper and it looks like a good quality.

@jasonjgill
Copy link
Contributor Author

jasonjgill commented Sep 3, 2024 via email

@jasonjgill
Copy link
Contributor Author

jasonjgill commented Sep 4, 2024 via email

Added back cpt_intersect_adjacent and cpt_related_genomes_prot for repeatability issues with comments, removed phageterm, phagetermvirome and defense_finder, will deal with these in a future PR. Net change is adding 3 new CPT tools.
@jasonjgill
Copy link
Contributor Author

Made a new commit to add back the 2 deleted tools (with comments) and removed the third-party tools. Net change now is just the 3 new CPT tools.

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.

2 participants