search

sib-swiss / pftools3

A suite of tools to build and search generalized profiles
GNU General Public License v2.0
10 stars 7 forks source link

pfscan errors are not catched #9

Closed duboism closed 4 years ago

duboism commented 4 years ago

As reported here, ps_scan.pl doesn't check if pfscan properly executed.

This is because open2, doesn't detect errors produced by the child. I would suggest to use IPC::Run which seems to handle this quite simply.

euphemizm commented 4 years ago

made a pull request for an utd ps_scan.pl version with do_profile_scan method compatible with pfscanV3 >=3.2 + added a check for child error ($?) after waitpid ('didn't even knew/realised that perl ps_scan.pl was included within pftools v3!)

smoretti commented 4 years ago

Thanks @euphemizm, pull request merged. @duboism could you retry?

duboism commented 4 years ago

Sorry for the delay. My idea was to build the code and replace pfscan with a script that just return 1 and see if it's detected. However, I'm not sure how can I launch ps_scan.pl.

I have tried ./src/Perl/ps_scan.pl -d ./data/FamilyProfile/sh2.prf test.fna (from the base directory): I see no output but the return code is 141 (which is not the case when I use the compiled pfscan). I was exepecting to see the error messages added in the commit.

duboism commented 4 years ago

OK, after some trials and error, I was able to test the new ps_scan and compare it to d37b3754230a1255d0f300c0dc24dac39d30e526 (I use the prosite motif file from InterPro and a fasta file):

So I think that we can say it works.

However, IIUC, $! is the perl variable for errno which is only meaningfull for syscalls. When calling a regular program fails, I would print the return code (and maybe capture stderr and print it). Note sure how to do it in Perl.

euphemizm commented 4 years ago

p.s. by default ps_scan.pl expects pfscan "v1" (the fortran version) pfscan should be found in the current PATH, or the path+progname could be specified via some option:

--pfscan : pathname to pfscan executable (if not defined will be 'pfscan', so executable has to be found within PATH env). This option could be used e.g. to use pftool v3 pfscanV3.

if ps_scan.pl sees "V3" in the path+progname, it will then use it with the appropriate v3 options (I know, it's...)

p.s. use ps_scan.pl if you want to scan prosite patterns (& profiles) (and / or have prosite "post-processing" between match results), but if you want to (or if it is enough 4u to) scan only against prosite profiles you could just use pfscan(V3)

duboism commented 4 years ago

@euphemizm Thanks for your answer but I'm not sure to understand what you mean (I don't know about the biological meaning of pftools, I am just an humble developer ;).

I think that I found a way to test that pfscan errors are caught by ps_scan.pl now by forcing pfscan to fail. The errors are caught so this is a good news. Do you agree on the method (i.e. replacing pfscan by a script that return non-0 to see if ps_scan detects it) ? It was based on the FORTRAN version of pfscan (because it's what we use) but I will do the same with pfscanV3 to be sure.

The last part of my answer was an humble suggestion (as I'm not familiar with Perl) on how to inform the user about the failure.

euphemizm commented 4 years ago

'just did a pull request for "sub do_profile_scan; when using pipe (IPC::Open2), upon child process errors show (child) return code '$?' (instead of system error str '$!')" ...

smoretti commented 4 years ago

Merged in master branch

duboism commented 4 years ago

Thanks for the correction. The error message is now Could not execute pfscan -flxz -v -C-1 - /home/mdubois/Code/interproscan/./core/io/src/test/resources/data/prosite/prosite.dat; returned: 256 at /home/mdubois/Code/pftools3/build/bin/ps_scan.pl line 1780. which is what is expected (as $? is the exit code of the process - 1 in this case - shifted by 8).

So this is issue is closed.

duboism commented 4 years ago

Just a question: do you plan to release a new version including this update soon ?

smoretti commented 4 years ago

I have just released version 3.2.5 to freeze ps_scan fix for interproscan