krasileva-group / plant_rgenes

12 stars 9 forks source link

Improvements to run_pfam_scan.sh #10

Closed peterjc closed 6 years ago

peterjc commented 6 years ago

Currently it seems the user is expected to edit run_pfam_scan.sh to set the directory where the Pfam database can be found, instead default to HMMER's convention of $HMMERDB

Also currently it seems the third-party script pfam_scan.pl is expected to be in the current directory, instead assume it is installed and on $PATH.

Note installing pfam_scan.pl version 1.6 via the BioConda package I wrote will do this,

$ conda install -c bioconda pfam_scan

See https://anaconda.org/bioconda/pfam_scan

peterjc commented 6 years ago

Note these two changes may cause minor difficulties to anyone who has already managed to get run_pfam_scan.sh to work on their local setup.

peterjc commented 6 years ago

I think something went wrong with the merge, this is wrong (old logic assuming pfam_scan.pl is in the current folder and calling it via perl):

CMDSTR+="'time perl pfam_scan.pl -e_seq 1 -e_dom 1 -as -outfile $dirname/pfam/${basename}_pfamscan-$(date +%m-%d-%Y).out -cpu 8 -fasta $FILE -dir $Pfam"

Should be as follows (new approach assuming pfam_scan.pl is installed, i.e. executable and on $PATH):

CMDSTR+="'time pfam_scan.pl -e_seq 1 -e_dom 1 -as -outfile $dirname/pfam/${basename}_pfamscan-$(date +%m-%d-%Y).out -cpu 8 -fasta $FILE -dir $Pfam"

Perhaps I should have submitted one pull request with this and the removal of the -pfamB option together to avoid the merge conflict. Sorry, I must have thought the two changes were independent.

krasileva commented 6 years ago

Hi Peter,

Ok, we will fix this.

Best wishes,

Ksenia


Ksenia Krasileva Group Leader

[cid:image001.png@01D36533.FA572930]

Norwich Research Park Norwich Norfolk NR4 7UZ +44 (0) 1603 450 2946 Ksenia.Krasileva@earlham.ac.ukmailto:Ksenia.Krasileva@earlham.ac.uk

www.earlham.ac.uk/krasileva-grouphttp://www.earlham.ac.uk/krasileva-group www.tsl.ac.uk/groups/krasileva-grouphttp://www.tsl.ac.uk/groups/krasileva-group @kseniakrasileva

www.earlham.ac.ukhttp://www.earlham.ac.uk/ www.tsl.ac.ukhttp://www.tsl.ac.uk/

[cid:image002.png@01D36533.FA572930]https://twitter.com/EarlhamInst[cid:image003.png@01D36533.FA572930]https://www.facebook.com/EarlhamInst/

From: Peter Cock notifications@github.com Reply-To: krasileva-group/plant_rgenes reply@reply.github.com Date: Friday, 24 November 2017 at 14:52 To: krasileva-group/plant_rgenes plant_rgenes@noreply.github.com Cc: "Ksenia Krasileva (EI)" Ksenia.Krasileva@earlham.ac.uk, State change state_change@noreply.github.com Subject: Re: [krasileva-group/plant_rgenes] Improvements to run_pfam_scan.sh (#10)

I think something went wrong with the merge, this is wrong (old logic assuming pfam_scan.pl is in the current folder and calling it via perl):

CMDSTR+="'time perl pfam_scan.pl -e_seq 1 -e_dom 1 -as -outfile $dirname/pfam/${basename}_pfamscan-$(date +%m-%d-%Y).out -cpu 8 -fasta $FILE -dir $Pfam"

Should be as follows (new approach assuming pfam_scan.pl is installed, i.e. executable and on $PATH):

CMDSTR+="'time pfam_scan.pl -e_seq 1 -e_dom 1 -as -outfile $dirname/pfam/${basename}_pfamscan-$(date +%m-%d-%Y).out -cpu 8 -fasta $FILE -dir $Pfam"

Perhaps I should have submitted one pull request with this and the removal of the -pfamB option together to avoid the merge conflict. Sorry, I must have thought the two changes were independent.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/krasileva-group/plant_rgenes/pull/10#issuecomment-346845065, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AB2D2Cj-lzKfNmpPUnboI5IgXJ1SIk6Xks5s5tgEgaJpZM4QkVaO.

peterjc commented 6 years ago

Great. Sorry, with hindsight I'd have avoided creating this merge conflict.