Altered if clasuses around selection of commands in umi_tools.py

[IanSudbery]

Fixes (or at least starts to fix) #536

@TomSmithCGAT Bit confused as to what is going on in the umi_tools module. Specifically what the original purpose of the if clause at lines 51-52 was.

I've also made each tool print its usage message if no options are passed at all. Don't know if this was the right was to do this, or if I should have done something more central (although that would become a problem if there was a tool that could be run without options. Which currently there isn't).

[TomSmithCGAT]

Hey @IanSudbery. Assuming you're referring to the below and the line numbers are from this PR? https://github.com/CGATOxford/UMI-tools/blob/0c72fa99c518088aa4505ee16721f85ac42eb0c2/umi_tools/umi_tools.py#L53-L54

I'm not sure what was going on with that elif either. It looks like I added it when we split into multiple tools to print help when a tool is specified and then you changed the output to be the version number (0101494bf236a485cbb14a55255113374df3c9a9 ). Regardless, each tool command line has help options added so no need to catch it in umi_tools.py.

I'm fine with having the same code in each tool to output help if no options are given. I agree it would be best done centrally though if you can be bothered recoding. I guess the easiest way to do it would be to catch instances where len(sys.args)==2 after the checks for version/help options, print the line you have about needing to add required options and then add --help to the sys.args and call the tool module main. I can't imagine us adding a tool with no help in the future since our base arg parser adds --help anyway

[TomSmithCGAT]

Can't see how to comment on lines that haven't been updated in the PR but L46 in umi_tools.py:

elif len(argv) == 1 or argv[1] == "--version" or argv[1] == "-v":

That len(argv) == 1 can go, since that case is met by the if and causes the help to be printed.

Minor thing.

[IanSudbery]

can't imagine us adding a tool with no help in the future since our base arg parser adds --help anyway

Thats not what I meant. I guess I was thinking about a tool that had no required arguements. For example, you could imagine that if extract had a default --pattern it could be run:

cat my_reads.fastq | umi_tools extract > my_reads.extracted.fastq

Although of course it doesn't, and I think all our tools require options of some sort (although come to think of it the new prepare_for_rsem or whatever we are going to call it might not).

In the above len(argv) == 1, but there is nothing wrong with it. Although perhaps this is just premature optimisation on my part.

I'll clean up the rough tabs.

[TomSmithCGAT]

Ah, got it. Yeah, that is a good point. Probably being optimistic about future tools being added 😉

[IanSudbery]

Whitespace should be tidied up.

[TomSmithCGAT]

Noticed in passing that a lot of the changes to umi_tools.py are also in #506. Should we first merge that one so that the contributions include @[epruesse]?

[IanSudbery]

yes Ian Sudbery Senior Lecturer in Bioinformatics, Sheffield Institute for Nucleic Acids, School of Biosciences, The University of Sheffield.

web: www.sudlab.co.uk Tel: 0114 222 2738 Twitter: IanSudbery Show Calendar Availability https://calendar.google.com/calendar/u/0?cid=aS5zdWRiZXJ5QHNoZWZmaWVsZC5hYy51aw

On Wed, 8 Jun 2022 at 14:31, Tom Smith @.***> wrote:

Noticed in passing that a lot of the changes to umi_tools.py are also in

506 https://github.com/CGATOxford/UMI-tools/pull/506. Should we first

merge that one so that the contributions include @[epruesse]?

— Reply to this email directly, view it on GitHub https://github.com/CGATOxford/UMI-tools/pull/537#issuecomment-1149920578, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJELDS3HG4H2VSPTZ4G5FDVOCOEBANCNFSM5X5LFZZA . You are receiving this because you were mentioned.Message ID: @.***>

[TomSmithCGAT]

@IanSudbery - This is OK to merge, right?