Re: [PATCH] gnu: Add metabat.

[Mark H Weaver]

Ben Woodcroft <address@hidden> writes:

> From 3f253f059edee5ee2fdb116e310ab210646acf40 Mon Sep 17 00:00:00 2001
> From: Ben Woodcroft <address@hidden>
> Date: Sat, 11 Jul 2015 16:49:30 +1000
> Subject: [PATCH] gnu: Add metabat.
>
> * gnu/packages/bioinformatics.scm (metabat): New variable.
> ---
>  gnu/packages/bioinformatics.scm | 72 
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
>
> diff --git a/gnu/packages/bioinformatics.scm b/gnu/packages/bioinformatics.scm
> index a28832d..a349d39 100644
> --- a/gnu/packages/bioinformatics.scm
> +++ b/gnu/packages/bioinformatics.scm
> @@ -1394,6 +1394,78 @@ resolution of binding sites through combining the 
> information of both
>  sequencing tag position and orientation.")
>      (license license:bsd-3)))
>  
> +(define-public metabat
> +  (package
> +    (name "metabat")
> +    (version "0.26.1")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append
> +                    "https://bitbucket.org/berkeleylab/metabat/get/";
> +                    version ".tar.bz2"))
> +              (file-name (string-append name "-" version ".tar.gz"))

The file-name should end with ".bz2", not ".gz".

> +              (sha256
> +               (base32
> +                "0vgrhbaxg4dkxyax2kbigak7w0arhqvw0szwp6gd9wmyilc44kfa"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:tests? #f ; tests run as part of build phase

It's a nitpick, but in this case it might be marginally nicer to just
delete the 'check' phase, and to put a comment above the (delete 'check)
and (delete 'install) explaining that the tests and install are both
done as part of the build phase.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (replace 'configure
> +                  (lambda* _
> +                    (substitute* "SConstruct" ; Do not distribute README
> +                      (("env.Install.idir_prefix, 'README.md'.") ""))

"env\\.Install\\(idir_prefix, 'README\\.md'\\)"

> +                    #t))
> +         (replace 'build
> +                  (lambda* (#:key inputs outputs #:allow-other-keys)
> +                    (substitute* "SConstruct"
> +                      (("/include/bam/bam.h")
> +                       "/include/samtools/bam.h"))
> +                    (substitute* "src/BamUtils.h"
> +                      (("#include .bam/bam.h.")
> +                       "#include \"samtools/bam.h\""))
> +                    (substitute* "src/BamUtils.h"
> +                      (("#include .bam/sam.h.")
> +                       "#include \"samtools/sam.h\""))
> +                    (substitute* "src/KseqReader.h"
> +                      (("#include \"bam/kseq.h\"")
> +                       "#include \"samtools/kseq.h\""))

Please move these to a different phase, maybe named 'fix-includes'.
Also, please use \\. in the patterns to match a literal period, put ^ to
anchor them to the start of a line, and use \" to match the start+end
quotes in the includes.  Finally, please combine the two substitute*
calls for "src/BamUtils.h" into a single call.  So, maybe something like
this: (untested)

--8<---------------cut here---------------start------------->8---
         (add-after 'unpack 'fix-includes
                    (lambda _
                      (substitute* "SConstruct"
                        (("/include/bam/bam\\.h")
                         "/include/samtools/bam.h"))
                      (substitute* "src/BamUtils.h"
                        (("^#include \"bam/bam\\.h\"")
                         "#include \"samtools/bam.h\"")
                        (("^#include \"bam/sam\\.h\"")
                         "#include \"samtools/sam.h\""))
                      (substitute* "src/KseqReader.h"
                        (("^#include \"bam/kseq\\.h\"")
                         "#include \"samtools/kseq.h\""))
                      #t))
--8<---------------cut here---------------end--------------->8---

> +         (delete 'install))))
> +    (inputs
> +     `(("zlib" ,zlib)
> +       ("perl" ,perl)
> +       ("samtools" ,samtools)
> +       ("htslib" ,htslib)))
> +    (native-inputs
> +     `(("boost" ,boost)

boost should be in 'inputs', not 'native-inputs'.

> +       ("scons" ,scons)))
> +    (home-page "https://bitbucket.org/berkeleylab/metabat";)
> +    (synopsis "Reconstruction of single genomes from complex microbial 
> communities")

Please move the string to the next line, to avoid running over 80
columns.

> +    (description
> +     "Grouping large genomic fragments assembled from shotgun metagenomic
> +sequences to deconvolute complex microbial communities, or metagenome 
> binning,
> +enables the study of individual organisms and their interactions.  MetaBAT is
> +an automated metagenome binning software, which integrates empirical
> +probabilistic distances of genome abundance and tetranucleotide frequency.")
> +   (license (license:non-copyleft "file://license.txt"
> +                                  "See LICENSE in the distribution."))))

s/LICENSE/license.txt/

Can you send an updated patch?

     Thanks,
       Mark