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

Update deprecated and rename #137

Merged
merged 9 commits into from
Oct 4, 2024

Conversation

kostrykin
Copy link
Member

Change tool directory names so that they correspond to the name specified in .shed.yml.

Move several tools to /deprecated.

util/README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@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.

The movements looks good to me, the linting a bit to harsh :)

Feel free to merge.

Co-authored-by: Björn Grüning <bjoern@gruenings.eu>
@kostrykin kostrykin merged commit 5bf3d88 into BMCV:master Oct 4, 2024
8 of 12 checks passed
@kostrykin kostrykin deleted the update-deprecated-and-rename branch October 4, 2024 10:33
@bernt-matthias
Copy link
Contributor

bernt-matthias commented Oct 24, 2024

Can you tell us why these tools (e.g. points2binaryimage) have been deprecated? Ping @rmassei

Deprecating tools in Galaxy seems still annoying to me, since they are still installable from toolshed and won't be uninstalled (or moved to a deprecated section) from any Galaxy (as far as I know).

Edit: Also for reproducibility we can not uninstall them anyway. My approach at the moment is to move them to another section (Deprecated tools) .... but unfortunately sections are not shown by default at tool search. Adding tags might be an options, but this seems super difficult to automatize with ansible / github....

@bernt-matthias
Copy link
Contributor

Hrm, actually it seems to be gone from the TS, what is going on here?

@bernt-matthias
Copy link
Contributor

@rmassei
Copy link
Contributor

rmassei commented Oct 24, 2024

It seems to me that points2binaryimage is now points2labelimage since they have the same functions and it was not present before on the TS.

@kostrykin
Copy link
Member Author

kostrykin commented Oct 24, 2024

Can you tell us why these tools (e.g. points2binaryimage) have been deprecated? Ping @rmassei

The functionality of the points2binaryimage tool has been merged into points2labelimage: #136 there are several reasons for this listed in the PR, but as @rmassei pointed out, the main reason is that their functionality was just very similar, and points2labelimage is more general.

I was not expecting any inconvenience due to this step, because to my understanding a deprecated tool can still be used, so existing workflows won't be harmed. Was I wrong?

@bgruening Can you maybe enlighten us what the exact consequences of depreacation are, and whether deprecation and hiding are disjoint actions or not?

@kostrykin
Copy link
Member Author

kostrykin commented Oct 24, 2024

Hrm, actually it seems to be gone from the TS, what is going on here?

@bernt-matthias: points2binaryimage is still in the toolshed: https://toolshed.g2.bx.psu.edu/repository/view_repository?sort=name&operation=view_or_manage_repository&id=87a558d17a053de7

@kostrykin
Copy link
Member Author

Regarding the other tools that have been moved to /deprecated:

  • roi2binaryimage was a directory that didn't contain any tools. It looked like a tool implementation that was started by someone long ago and never finished. But since I wasn't 100% sure, I didn't want to just delete it.
  • 3d_tensor_feature_dimension_reduction was not installed on Galaxy EU and we didn't plan to continue support for this tool. If the functionality is required, we would rather implement a more general tool (e.g., not restricted for 3D).
  • curl_post was deprecated because that the tool was unmaintained for many years and actually unrelated to image analysis. It was not intended to hide this tool.

I'm overly confused by the current state of the curl_post tool. Background: I asked @bgruening to mark curl_post as deprecated in #93. Now it seems that curl_post is hidden (I can't find it on Galaxy EU anymore), but there is no indication of it being deprecated in the toolshed: https://toolshed.g2.bx.psu.edu/repository/view_repository?sort=name&operation=view_or_manage_repository&id=6ae3d5066a82abe8 @bgruening Do you maybe know what is going on here?

@bernt-matthias
Copy link
Contributor

The functionality of the points2binaryimage tool has been merged into points2labelimage: #136 there are several reasons for this listed in the PR, but as @rmassei pointed out, the main reason is that their functionality was just very similar, and points2labelimage is more general.

Makes sense. Thanks for the explanations.

I was not expecting any inconvenience due to this step, because to my understanding a deprecated tool can still be used, so existing workflows won't be harmed. Was I wrong?

True.

@bgruening Can you maybe enlighten us what the exact consequences of depreacation are, and whether deprecation and hiding are disjoint actions or not?

There is currently no systematic/automatic mechanism. At the moment tools can be manually marked as deprecated in the TS. Admins could crawl this info, but I'm not aware of any automatism marking installed tools as deprecated on any instance (but maybe there is).

Lets see if other tool devs / admins have ideas galaxyproject/galaxy#19053

points2binaryimage is still in the toolshed: https://toolshed.g2.bx.psu.edu/repository/view_repository?sort=name&operation=view_or_manage_repository&id=87a558d17a053de7

I see. It's just the search that does not find them anymore.

@bgruening
Copy link
Collaborator

There is currently no systematic/automatic mechanism. At the moment tools can be manually marked as deprecated in the TS. Admins could crawl this info, but I'm not aware of any automatism marking installed tools as deprecated on any instance (but maybe there is).

I don't think there is, and I don't think there should be.
A notification to the admin maybe, but not hiding them automatically. But if you ping an admin, the admin also needs to know the reason .... a rabbit hole ...

Do you maybe know what is going on here?

This tool belong to Thomas, I can not deprecate it.

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.

4 participants