search

peterjc / pico_galaxy

Galaxy tools and wrappers for sequence analysis
17 stars 25 forks source link

Update signalp to 4.1 #25

Open abretaud opened 6 years ago

abretaud commented 6 years ago

Hi, As one of our users had problems with the signalp3.0 output, I had to update it to version 4.1. Here's the modified xml and py The doc and tests would need to be updated too, but I don't really have the time right now... I thought you could be interested anyway Cheers

peterjc commented 6 years ago

Thanks Anthony, I'm not planning to work on this right away, but having the pull request here is useful.

I'd prefer to have the SignalP 3 and 4 wrappers as two separate tools, and related to that was wondering about splitting this suite of tools which are currently a single Tool Shed entry into their own independent Tool Shed entries. To that end, starting SignalP 4 as a new folder of its own makes sense.

I've deliberately not worked on this because SignalP (and TMHMM etc) are not open source so I cannot setup automated testing. This means any work on the tools becomes much more work due to manual setups for testing being essential. Sigh.

I even contacted the tool authors to ask about this - for example permission to redistribute the software for testing, but this was denied.

Also, what was going wrong with SignalP 3? We're still using that locally and recently ran into #24 following some changes to our cluster and how we run jobs.

abretaud commented 6 years ago

I've deliberately not worked on this because SignalP (and TMHMM etc) are not open source so I cannot setup automated testing. This means any work on the tools becomes much more work due to manual setups for testing being essential. Sigh.

Yes, that's also the reason I made this in a quick and dirty way

The problem I had with signalp3 was this error which was thrown with some of our test data. It's hard to investigate without the sources, and as 4.1 doesn't have this problem, I just updated and people are happy now :)

peterjc commented 6 years ago

Right - SignalP v3 seems to be two binary tools with a perl wrapper which doesn't check the two tools return the same number of results. Sometimes one seems to skip a sequence, leaving the output file messed up. I don't think I ever reduced this to a clear test case, but that assert was to guard against this.

peterjc commented 6 years ago

Note to self, the RXLR wrapper expects the old columns (using Python counting, 0 = ID, 18 = HMM_Sprob_score and 5 = NN_Ymax_pos), and would also need updating if used with this Signalp 4.1 wrapper.

savipuray commented 5 years ago

Hi,

Thanks for this Galaxy toolshed. It is really helpful for effector analysis. I updated my toolshed to SignalP version 4.1, which is working perfectly. I was wondering if there was an updated RxLR wrapper that works for SignalP4.1. If not, I was wondering how to modify the existing wrapper for the purpose. Sorry if this is a dumb question. I am new to this and any help would be much appreciated. Thank you!

Savithri

peterjc commented 5 years ago

The RXLR tool used the same version of SignalP as the tools cited, so in that sense it doesn't need changing.

If you want to apply the same ideas but with SignalP v4.1, then as per my earlier comment the parser needs updating as they changed the output. I have not sat down to do this: https://github.com/peterjc/pico_galaxy/pull/25#issuecomment-349022098

I would strongly recommend you have both the old and the new SignalP installed, and use the old one with the RXLR tool.

savipuray commented 5 years ago

Thank you! That helps!