Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

FastTree, Glimmer3, tRNAscan-SE #57

Closed
wants to merge 23 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

bosborne commented Jan 7, 2013

FastTree can handle alignments with up to a million of sequences in a reasonable amount of time and memory. Glimmer3 (genes) and tRNAscan-SE (tRNAs) are standard prokaryotic genome prediction tools. All have tests.

@samueljohn samueljohn commented on an outdated diff Jan 11, 2013

glimmer302a.rb
+
+ end
+
+ def test
+ Dir.glob("#{lib}/test.*") do |out|
+ system "rm -f #{out}"
+ end
+
+ cd lib do
+ system "g3-from-scratch.csh #{lib}/tpall.fna test"
+ end
+
+ if FileTest.exists? "#{lib}/test.predict"
+ %x(diff #{lib}/test.predict #{lib}/from-scratch.predict).empty? ? system("true") : system("false")
+ else
+ system "false"
@samueljohn

samueljohn Jan 11, 2013

Contributor

Don't need system here. Just false is enough.

Contributor

samueljohn commented Jan 11, 2013

Thanks for your work here. Looks already good.
The tests should not write output into lib. Perhaps mktemp or something?
The system("false") was just in the template as place holder. I will review in more detail next week.

Contributor

bosborne commented Jan 12, 2013

Samuel,

Made those suggested changes, but note that I'm doing 'require "tmpdir"'. I'm guessing that that's OK.

Not a Ruby person yet! Just getting started.

Brian O.

On Jan 11, 2013, at 4:22 PM, Samuel John notifications@github.com wrote:

Thanks for your work here. Looks already good.
The tests should not write output into lib. Perhaps mktemp or something?
The system("false") was just in the template as place holder. I will review in more detail next week.


Reply to this email directly or view it on GitHub.

@samueljohn samueljohn and 1 other commented on an outdated diff Jan 12, 2013

glimmer302a.rb
+require 'tmpdir'
+
+class Glimmer302a < Formula
+ homepage 'http://www.cbcb.umd.edu/software/glimmer/'
+ url 'http://www.cbcb.umd.edu/software/glimmer/glimmer302a.tar.gz'
+ sha1 '27fbd2498f997e0a47026b348b2fc95b073b712a'
+ version '3.02'
+
+ def install
+ cd 'src' do
+ system 'make'
+ end
+
+ cd 'bin' do
+ bin.install %w[window-acgt start-codon-distrib long-orfs entropy-score build-fixed uncovered
+ score-fixed glimmer3 entropy-profile anomaly multi-extract extract build-icm]
@samueljohn

samueljohn Jan 12, 2013

Contributor

Are all these binaries needed, or do you just call one main binary and the others are internally called? I am asking because "extract" may perhaps be a fairly common name and may conflict when it is symlinked into $(brew --prefix)/bin.

@bosborne

bosborne Jan 13, 2013

Contributor

The glimmer3 package has a bunch of binaries and 3 shell scripts. The "extract" app is called by all the shell scripts, but it does not necessarily have to be in $PATH. Here are the relevant lines from the shell scrips:

/usr/local/bin/g3-from-scratch.csh:$glimmerpath/extract -t $genome $tag.longorfs > $tag.train
/usr/local/bin/g3-from-scratch.csh: echo "Failed to extract training sequences"
/usr/local/bin/g3-from-training.csh: echo "in file to extract a training set. Use to prefix"
/usr/local/bin/g3-from-training.csh:$glimmerpath/extract -t $genome $coords > $tag.train
/usr/local/bin/g3-from-training.csh: echo "Failed to extract training sequences"
/usr/local/bin/g3-from-training.csh: | $glimmerpath/extract $genome - > $tag.upstream
/usr/local/bin/g3-iterated.csh:$glimmerpath/extract -t $genome $tag.longorfs > $tag.train
/usr/local/bin/g3-iterated.csh: echo "Failed to extract training sequences"
/usr/local/bin/g3-iterated.csh: | $glimmerpath/extract $genome - > $tag.upstream

All the apps, including glimmer3 itself, are expected to be in $glimmerpath, e.g.:

/usr/local/bin/g3-from-training.csh:$glimmerpath/glimmer3 $glimmeropts -b $tag.motif -P $startuse $genome $tag.icm $tag

Personally I never use the apps directly, I always use the shell scripts. Should we put all the apps in lib and set $glimmerpath to the lib location?

On Jan 12, 2013, at 3:20 AM, Samuel John notifications@github.com wrote:

In glimmer302a.rb:

+require 'tmpdir'
+
+class Glimmer302a < Formula

  • def install
  • cd 'src' do
  •  system 'make'
    
  • end
  • cd 'bin' do
  •  bin.install %w[window-acgt start-codon-distrib long-orfs entropy-score build-fixed uncovered
    
  •   score-fixed glimmer3 entropy-profile anomaly multi-extract extract build-icm]
    
    Are all these binaries needed, or do you just call one main binary and the others are internally called? I am asking because "extract" may perhaps be a fairly common name and may conflict when it is symlinked into $(brew --prefix)/bin.


Reply to this email directly or view it on GitHub.

@samueljohn samueljohn and 1 other commented on an outdated diff Jan 12, 2013

glimmer302a.rb
@@ -0,0 +1,46 @@
+require 'formula'
+require 'tmpdir'
+
+class Glimmer302a < Formula
+ homepage 'http://www.cbcb.umd.edu/software/glimmer/'
+ url 'http://www.cbcb.umd.edu/software/glimmer/glimmer302a.tar.gz'
+ sha1 '27fbd2498f997e0a47026b348b2fc95b073b712a'
+ version '3.02'
@samueljohn

samueljohn Jan 12, 2013

Contributor

Often homebrew can extract the version from the url. In this case perhaps "302" or "302a" wich would both be fine. When we upgrade it's easier to let the automatic to the work -- if possible.

@bosborne

bosborne Jan 13, 2013

Contributor

Samuel,

I tried:

version Version.parse(url)
version Version.parse(url).to_s

They both give me "Error: Invalid @Version". Your thoughts?

Brian O.

On Jan 12, 2013, at 3:21 AM, Samuel John notifications@github.com wrote:

In glimmer302a.rb:

@@ -0,0 +1,46 @@
+require 'formula'
+require 'tmpdir'
+
+class Glimmer302a < Formula


Reply to this email directly or view it on GitHub.

@samueljohn

samueljohn Jan 15, 2013

Contributor

Sorry not to be clear. Just remove the version '3.02' line. Don't have to do anything special.

@samueljohn

samueljohn Jan 15, 2013

Contributor

I see. This seems a problem of homebrew's automatic. Sorry.

@bosborne

bosborne Jan 15, 2013

Contributor

Ah! Done.

On Jan 15, 2013, at 7:52 AM, Samuel John notifications@github.com wrote:

In glimmer302a.rb:

@@ -0,0 +1,46 @@
+require 'formula'
+require 'tmpdir'
+
+class Glimmer302a < Formula


Reply to this email directly or view it on GitHub.

@samueljohn samueljohn and 1 other commented on an outdated diff Jan 12, 2013

glimmer302a.rb
@@ -0,0 +1,46 @@
+require 'formula'
+require 'tmpdir'
+
+class Glimmer302a < Formula
@samueljohn

samueljohn Jan 12, 2013

Contributor

We would just name it Glimmer. Or, if there is already a Glimmer formula (say for the 2.x), we would call it Glimmer3, so both can be installed. Note upgrading to a future 303 would not work because the formula's name changed.

@bosborne

bosborne Jan 13, 2013

Contributor

Done, renamed to "glimmer3" (but various issues remain). There is a Glimmer2, so that would be a separate package, I think.

On Jan 12, 2013, at 3:23 AM, Samuel John notifications@github.com wrote:

In glimmer302a.rb:

@@ -0,0 +1,46 @@
+require 'formula'
+require 'tmpdir'
+
+class Glimmer302a < Formula
We would just name it Glimmer. Or, if there is already a Glimmer formula (say for the 2.x), we would call it Glimmer3, so both can be installed. Note upgrading to a future 303 would not work because the formula's name changed.


Reply to this email directly or view it on GitHub.

@samueljohn

samueljohn Jan 15, 2013

Contributor

where is that glimmer2 from? Is it a formula of some kind or the name of a software that is different from glimmer3?
Is it the same as for python and python3 ?

@bosborne

bosborne Jan 15, 2013

Contributor

glimmer2 comes from the same lab, but it's an older version of the package. I have never used glimmer2 but it was used for the same purpose (bacterial gene prediction).

On Jan 15, 2013, at 7:54 AM, Samuel John notifications@github.com wrote:

In glimmer302a.rb:

@@ -0,0 +1,46 @@
+require 'formula'
+require 'tmpdir'
+
+class Glimmer302a < Formula
where is that glimmer2 from? Is it a formula of some kind or the name of a software that is different from glimmer3?
Is it the same as for python and python3 ?


Reply to this email directly or view it on GitHub.

@samueljohn samueljohn and 1 other commented on an outdated diff Jan 12, 2013

glimmer302a.rb
+
+ lib.install Dir.glob('scripts/*.awk')
+ lib.install Dir.glob('sample-run/*.predict')
+ lib.install 'sample-run/tpall.fna'
+
+ Dir.glob('scripts/*.csh').each do |script|
+ inreplace script, '/fs/szgenefinding/Glimmer3/scripts', lib
+ inreplace script, '/fs/szgenefinding/Glimmer3', '/usr/local'
+ bin.install script
+ end
+
+ end
+
+ def test
+ tmp_dir = File.join(Dir::tmpdir, "#{Time.now.to_i}")
+ Dir.mkdir(tmp_dir)
@samueljohn

samueljohn Jan 12, 2013

Contributor

Does the mktemp do block not work here, as you have done for fasttree? Then, we would not need the require tmpdir.
The mktemp do automatically generates a new private dir and cds into it.

@bosborne

bosborne Jan 15, 2013

Contributor

Added 'mktemp' block, removed the 'require', put files used in test into share.

On Jan 12, 2013, at 3:25 AM, Samuel John notifications@github.com wrote:

In glimmer302a.rb:

  • lib.install Dir.glob('scripts/*.awk')
  • lib.install Dir.glob('sample-run/*.predict')
  • lib.install 'sample-run/tpall.fna'
  • Dir.glob('scripts/*.csh').each do |script|
  •  inreplace script, '/fs/szgenefinding/Glimmer3/scripts', lib
    
  •  inreplace script, '/fs/szgenefinding/Glimmer3', '/usr/local'
    
  •  bin.install script
    
  • end
  • end
  • def test
  • tmp_dir = File.join(Dir::tmpdir, "#{Time.now.to_i}")
  • Dir.mkdir(tmp_dir)
    Does the mktemp do block not work here, as you have done for fasttree? Then, we would not need the require tmpdir.
    The mktemp do automatically generates a new private dir and cds into it.


Reply to this email directly or view it on GitHub.

@samueljohn samueljohn and 1 other commented on an outdated diff Jan 12, 2013

glimmer302a.rb
+ end
+
+ end
+
+ def test
+ tmp_dir = File.join(Dir::tmpdir, "#{Time.now.to_i}")
+ Dir.mkdir(tmp_dir)
+
+ cd tmp_dir do
+ system "g3-from-scratch.csh #{lib}/tpall.fna test"
+ end
+
+ if FileTest.exists? "#{tmp_dir}/test.predict"
+ %x(diff #{tmp_dir}/test.predict #{lib}/from-scratch.predict).empty? ? "true" : "false"
+ else
+ "false"
@samueljohn

samueljohn Jan 12, 2013

Contributor
Python 🐍 ❤️ Ruby explanation
False false boolean value
"false" "false" The string with content "false". if "false" is true.
@bosborne

bosborne Jan 13, 2013

Contributor

I see. Use:

true
false

No quotes. Thanks for that, correct me if I'm wrong.

On Jan 12, 2013, at 3:30 AM, Samuel John notifications@github.com wrote:

In glimmer302a.rb:

  • end
  • end
  • def test
  • tmp_dir = File.join(Dir::tmpdir, "#{Time.now.to_i}")
  • Dir.mkdir(tmp_dir)
  • cd tmp_dir do
  •  system "g3-from-scratch.csh #{lib}/tpall.fna test"
    
  • end
  • if FileTest.exists? "#{tmp_dir}/test.predict"
  •  %x(diff #{tmp_dir}/test.predict #{lib}/from-scratch.predict).empty? ? "true" : "false"
    
  • else
  •  "false"
    
    Python Ruby explanation
    False false boolean value
    "false" "false" The string with content "false". if "false" is true.

    Reply to this email directly or view it on GitHub.

@samueljohn samueljohn commented on the diff Jan 12, 2013

trnascan.rb
+
+class Trnascan < Formula
+ homepage 'http://selab.janelia.org/tRNAscan-SE/'
+ url 'http://selab.janelia.org/software/tRNAscan-SE/tRNAscan-SE.tar.Z'
+ sha1 'fd2db5b1bb059dfdcf0fced1c865909da601d71f'
+ version '1.23'
+
+ def install
+ inreplace 'makefile' do |s|
+ s.change_make_var! 'CFLAGS', '-D_POSIX_C_SOURCE=1'
+ s.change_make_var! 'LIBDIR', lib
+ s.change_make_var! 'BINDIR', bin
+ end
+ system 'make all'
+
+ bin.install %w[coves-SE covels-SE eufindtRNA trnascan-1.4]
@samueljohn

samueljohn Jan 12, 2013

Contributor

why is the binary called trnascan-1.4 when the version is 1.23?

@bosborne

bosborne Jan 13, 2013

Contributor

Odd, right? The version of the package is 1.23. The version of the trnascan binary in the package is 1.4. Other binaries in the package include coves and covels-SE, but their names contain no version numbers.

The name of the shell script that you actually use is "tRNAscan-SE".

On Jan 12, 2013, at 3:31 AM, Samuel John notifications@github.com wrote:

In trnascan.rb:

+class Trnascan < Formula

  • def install
  • inreplace 'makefile' do |s|
  •  s.change_make_var! 'CFLAGS', '-D_POSIX_C_SOURCE=1'
    
  •  s.change_make_var! 'LIBDIR', lib
    
  •  s.change_make_var! 'BINDIR', bin
    
  • end
  • system 'make all'
  • bin.install %w[coves-SE covels-SE eufindtRNA trnascan-1.4]
    why is the binary called trnascan-1.4 when the version is 1.23?


Reply to this email directly or view it on GitHub.

@samueljohn samueljohn and 1 other commented on an outdated diff Jan 12, 2013

trnascan.rb
+ def install
+ inreplace 'makefile' do |s|
+ s.change_make_var! 'CFLAGS', '-D_POSIX_C_SOURCE=1'
+ s.change_make_var! 'LIBDIR', lib
+ s.change_make_var! 'BINDIR', bin
+ end
+ system 'make all'
+
+ bin.install %w[coves-SE covels-SE eufindtRNA trnascan-1.4]
+ bin.install 'tRNAscan-SE.src'.sub(/\.src/, '')
+
+ lib.install Dir.glob('gcode.*')
+ lib.install Dir.glob('*.cm')
+ lib.install 'Demo/F22B7.fa'
+ lib.install 'testrun.ref'
+ lib.install Dir.glob('*signal')
@samueljohn

samueljohn Jan 12, 2013

Contributor

I have the feeling that we are installing here too much into lib, which will be symlinked into $(brew --prefix)/lib. We basically want only libraries there. Other stuff should go into (share/'trnascan').install especially the Demo, ref files and so forth...

We could also make up a new dir under prefix. For example prefix/'support' will not be symlinked into $(brew --prefix)/support.

@bosborne

bosborne Jan 15, 2013

Contributor

Yes, makes sense. So I've put those files used solely for testing into /usr/local/share/trnascan. Also put the test result into a tmp dir that's automatically removed.

On Jan 12, 2013, at 4:09 AM, Samuel John notifications@github.com wrote:

In trnascan.rb:

  • def install
  • inreplace 'makefile' do |s|
  •  s.change_make_var! 'CFLAGS', '-D_POSIX_C_SOURCE=1'
    
  •  s.change_make_var! 'LIBDIR', lib
    
  •  s.change_make_var! 'BINDIR', bin
    
  • end
  • system 'make all'
  • bin.install %w[coves-SE covels-SE eufindtRNA trnascan-1.4]
  • bin.install 'tRNAscan-SE.src'.sub(/.src/, '')
  • lib.install Dir.glob('gcode.*')
  • lib.install Dir.glob('*.cm')
  • lib.install 'Demo/F22B7.fa'
  • lib.install 'testrun.ref'
  • lib.install Dir.glob('*signal')
    I have the feeling that we are installing here too much into lib, which will be symlinked into $(brew --prefix)/lib. We basically want only libraries there. Other stuff should go into (share/'trnascan').install especially the Demo, ref files and so forth...

We could also make up a new dir under prefix. For example prefix/'support' will not be symlinked into $(brew --prefix)/support.


Reply to this email directly or view it on GitHub.

@samueljohn samueljohn and 1 other commented on an outdated diff Jan 12, 2013

trnascan.rb
+
+ bin.install %w[coves-SE covels-SE eufindtRNA trnascan-1.4]
+ bin.install 'tRNAscan-SE.src'.sub(/\.src/, '')
+
+ lib.install Dir.glob('gcode.*')
+ lib.install Dir.glob('*.cm')
+ lib.install 'Demo/F22B7.fa'
+ lib.install 'testrun.ref'
+ lib.install Dir.glob('*signal')
+
+ File.rename('tRNAscan-SE.man','tRNAscan-SE.1')
+ man1.install 'tRNAscan-SE.1'
+ end
+
+ def test
+ system "rm -f #{lib}/testrun.out"
@samueljohn

samueljohn Jan 12, 2013

Contributor

please also mktemp do here, so we avoid the rm of the test output.

@bosborne

bosborne Jan 15, 2013

Contributor

Done.

On Jan 12, 2013, at 4:10 AM, Samuel John notifications@github.com wrote:

In trnascan.rb:

  • bin.install %w[coves-SE covels-SE eufindtRNA trnascan-1.4]
  • bin.install 'tRNAscan-SE.src'.sub(/.src/, '')
  • lib.install Dir.glob('gcode.*')
  • lib.install Dir.glob('*.cm')
  • lib.install 'Demo/F22B7.fa'
  • lib.install 'testrun.ref'
  • lib.install Dir.glob('*signal')
  • File.rename('tRNAscan-SE.man','tRNAscan-SE.1')
  • man1.install 'tRNAscan-SE.1'
  • end
  • def test
  • system "rm -f #{lib}/testrun.out"
    please also mktemp do here, so we avoid the rm of the test output.


Reply to this email directly or view it on GitHub.

Contributor

samueljohn commented Jan 12, 2013

@bosborne no worries, I am not a Ruby person either :-)
I love homebrew even if it is written in Ruby and not because ^^

Please see my comments on the diff. I think the mktemp do block does not require require because brew did this already.

@samueljohn samueljohn commented on the diff Jan 15, 2013

fasttree.rb
@@ -0,0 +1,27 @@
+require 'formula'
+
+class Fasttree < Formula
+ homepage 'http://meta.microbesonline.org/fasttree/'
+ url 'http://meta.microbesonline.org/fasttree/FastTree.c'
+ version '2.1.3'
+ sha1 '371f12d6177822f20d240327b4cdfd7c4a6923e4'
+
+ def install
+ system "#{ENV.cc} -lm -O3 -finline-functions -funroll-loops -Wall -o FastTree FastTree.c"
+ bin.install "FastTree"
+ end
+
+ def test
+ mktemp do
@samueljohn

samueljohn Jan 15, 2013

Contributor

Recently, homebrew got an alternative way to do def test; mktemp do in one step.

test do
   <write temp files>
end
<things are cleaned up automatically>

But I can fix that when commiting - you don't have to. I learned about that quite recently.

Contributor

samueljohn commented Jan 15, 2013

Alright, I did some minor fixes and installed more into libexec instead of lib. The tests do all pass for me.
For fasttree, I had to add a fails_with :clang block because I got segfaults.

Can you please brew update an check if all the stuff works as you intended?

Thanks for the good work here! Pulled.

@samueljohn samueljohn closed this Jan 15, 2013

@ghost ghost assigned samueljohn Jan 15, 2013

Contributor

bosborne commented Jan 15, 2013

Samuel, everything installs and tests. Excellent!

On Jan 15, 2013, at 8:52 AM, Samuel John notifications@github.com wrote:

Alright, I did some minor fixes and installed more into libexec instead of lib. The tests do all pass for me.
For fasttree, I had to add a fails_with :clang block because I got segfaults.

Can you please brew update an check if all the stuff works as you intended?

Thanks for the good work here! Pulled.


Reply to this email directly or view it on GitHub.

ghost pushed a commit to rleigh-codelibre/homebrew-science that referenced this pull request Mar 18, 2015

Merge pull request #57 from sbesson/5.0.1
Bump OMERO and Bio-Formats formulas to 5.0.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment