galaxy icon indicating copy to clipboard operation
galaxy copied to clipboard

interval2maf remove from unversioned or deprecate

Open bernt-matthias opened this issue 1 year ago • 5 comments

In order to make the IUC version of the tool work (https://github.com/galaxyproject/tools-iuc/pull/4769) we need to

  • remove it from the list of unversioned tools (b7ad595b7298729924a0744fba62ba7a6198410a)
  • or just deprecate it (214d11c9214fc9a888db11bfc9aa186855a42fa4)

Intuitively I would suggest the later option since it is know to be broken and we have a working version at IUC (https://github.com/galaxyproject/tools-iuc/pull/4769 will be working after accepting this PR)

How to test the changes?

(Select all options that apply)

  • [ ] I've included appropriate automated tests.
  • [ ] This is a refactoring of components with existing test coverage.
  • [ ] Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • [x] I agree to license these contributions under Galaxy's current license.
  • [x] I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to [email protected].

bernt-matthias avatar Sep 06 '22 14:09 bernt-matthias

Can you go with https://github.com/galaxyproject/galaxy/commit/b7ad595b7298729924a0744fba62ba7a6198410a ? I'm not sure it's wise to remove tools, just for the potential loss of tool state.

mvdbeek avatar Sep 21 '22 09:09 mvdbeek

If we don't remove it, maybe at least move it to a deprecated/ subdirectory of tools, and remove it from the lib/galaxy/config/sample/tool_conf.xml.sample ? So people won't try to update it or add it to new Galaxy installations.

nsoranzo avatar Sep 21 '22 09:09 nsoranzo

Can you go with b7ad595 ? I'm not sure it's wise to remove tools, just for the potential loss of tool state.

Will do. How about a comment in the tool: stating that no more updates should be done?

bernt-matthias avatar Sep 22 '22 08:09 bernt-matthias

Can you go with b7ad595 ? I'm not sure it's wise to remove tools, just for the potential loss of tool state.

Will do. How about a comment in the tool: stating that no more updates should be done?

What about my suggestion? https://github.com/galaxyproject/galaxy/pull/14573#issuecomment-1253445618

nsoranzo avatar Sep 22 '22 08:09 nsoranzo

I think that if you've had the tool installed (and you've run it at least once) you should keep it installed, so moving the location is also going to break any tool_conf.xml that pointed at the original location.

It's tricky ... if the IUC tool didn't change any of the parameter names maybe it does make sense to drop the file. I'll leave that decision up to you.

mvdbeek avatar Sep 22 '22 08:09 mvdbeek

Btw. what exactly is "tool state"?

bernt-matthias avatar Sep 23 '22 15:09 bernt-matthias

I think that if you've had the tool installed (and you've run it at least once) you should keep it installed, so moving the location is also going to break any tool_conf.xml that pointed at the original location.

Ah, you're right, should have thought about that! So I guess we are left with:

  • From the second commit, keep only the changes to lib/galaxy/config/sample/tool_conf.xml.sample and lib/galaxy/tool_util/xsd/galaxy.xsd
  • Add a comment to tools/maf/interval2maf.xml stating that the tool is deprecated in favour of the tools-iuc one.

Btw. what exactly is "tool state"?

I guess Marius means the ability to re-run/inspect the metadata of old output datasets of this tool, plus not breaking old workflows.

nsoranzo avatar Sep 23 '22 15:09 nsoranzo

Thanks for the precise suggestions @nsoranzo

bernt-matthias avatar Sep 26 '22 13:09 bernt-matthias

This PR was merged without a "kind/" label, please correct.

github-actions[bot] avatar Oct 17 '22 09:10 github-actions[bot]