It contains multiple functions that are over 50 lines long.
It can contains a class (that should be able to simplify a few things) but in this class multiple methods are over 50 lines long.
A global log variable is defined
It overwrites the ConnectionError that is in standard library.
It has by far the highest complexity of all files in ephemeris (18)
Shed-tools is not pleasant to work at. Navigating through it is a jungle. It should be totally refactored.
I have a few suggestions how to approach this:
Take get_tool_list_from_galaxy.py as an example. This was done for run-data-managers as well, and as a result complexity was dropped, while at the same time increasing functionality.
Essentially this means shed_tools should be written as a class that:
Has a method to output currently installed tools
Has a method to check whether a tool is currently installed
Has a method to install a tool
Has a method to test a tool (Thanks a lot @jmchilton !)
Has some sort of install_tools method for multiple tools including logging.
The parsing of the tool list yaml may be moved to a separate file
Some stuff from get_tool_list from galaxy might be reused in shed-tools. (Like determining current tool list)
Some redundant command line options should be removed. Installing by passing a yaml string? OK. Installing by tool_list file? OK. Installing by specifying every frigging tool attribute on the commandline? We can drop that IMO.
This is something I have been thinking about doing it for a while. Just putting it out there before we increase the complexity of shed-tools even further. I think refactoring will also improve the ability to test using pytest #80.
rhpvorderman
commented
6 years ago
Shed-tools is not pleasant to work at. Navigating through it is a jungle. It should be totally refactored. I have a few suggestions how to approach this:
get_tool_list_from_galaxy.pyas an example. This was done forrun-data-managersas well, and as a result complexity was dropped, while at the same time increasing functionality. Essentially this means shed_tools should be written as a class that:This is something I have been thinking about doing it for a while. Just putting it out there before we increase the complexity of shed-tools even further. I think refactoring will also improve the ability to test using pytest #80.