Re: [PATCH 10/10] gnu: Add python-scikit-bio.

[Ricardo Wurmus]

Hi Ben,

thanks for your patch.

> * gnu/packages/python.scm (python-scikit-bio,
> python2-scikit-bio): New variables.

I think this should be on one line.  In any case, breaking the line so
soon looks a bit weird.

> ---
>  gnu/packages/python.scm | 91 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
>
> diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
> index 7aae6fc..b359422 100644
> --- a/gnu/packages/python.scm
> +++ b/gnu/packages/python.scm
> @@ -8945,6 +8945,97 @@ numbers (i.e. signed/unsigned floats or ints), or as 
> versions.")
>                   ("python2-enum34" ,python2-enum34)
>                   ("python2-setuptools" ,python2-setuptools)))))))
>  
> +(define-public python-scikit-bio
> +  (package
> +    (name "python-scikit-bio")
> +    (version "0.4.2")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append
> +             
> "https://pypi.python.org/packages/source/s/scikit-bio/scikit-bio-";
> +             version
> +             ".tar.gz"))
> +       (sha256
> +        (base32
> +         "06nrcgfz6c3jb2dnaf1wnvx3dyww94p454c4126gvcvfgv6scczy"))))
> +    (build-system python-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         ;; Install procedure installs extraneous binaries.
> +         (add-after 'install 'remove-extraneous-files
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out"))
> +                    (bin (string-append out "/bin")))
> +               (delete-file-recursively bin))
> +             #t))
> +         ;; `setup.py test' does not run tests, but there are many test
> +         ;; failures anyway

What does it mean that “setup.py test” does not run tests?  Is there
another test target?

> +         ;; e.g. https://github.com/biocore/scikit-bio/issues/1308.  Run a
> +         ;; simple import test instead.
> +         (delete 'check)
> +         (add-after 'install 'check-after-install
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (let ((pythonpath
> +                    (string-append
> +                     (getenv "PYTHONPATH")
> +                     ":" (assoc-ref outputs "out")
> +                     "/lib/python"
> +                     (string-take (string-take-right
> +                                   (assoc-ref inputs "python") 5) 3)
> +                     "/site-packages")))
> +               (setenv "PYTHONPATH" pythonpath)

I would not use a let here and replace “pythonpath” with the
“(string-append...)” for the new PYTHONPATH.

> +               (zero?
> +                (with-directory-excursion "ci"
> +                  (system*
> +                   (string-append
> +                    (assoc-ref inputs "python")
> +                    "/bin/python")

You can use “python” directly, or “(which "python")”.  The PATH should
include it at this point.

> +                   "-c"
> +                   (string-append "import matplotlib; "
> +                                  "matplotlib.use(\"Agg\"); "
> +                                  "import skbio"))))))))))
> +    (native-inputs
> +     `(("python-nose" ,python-nose)
> +       ("python-pep8" ,python-pep8)
> +       ("python-flake8" ,python-flake8)
> +       ("python-dateutil" ,python-dateutil-2)))
> +    (propagated-inputs
> +     `(("python-bz2file" ,python-bz2file)
> +       ("python-lockfile" ,python-lockfile)
> +       ("python-cachecontrol" ,python-cachecontrol)
> +       ("python-contextlib2" ,python-contextlib2)
> +       ("python-decorator" ,python-decorator)
> +       ("python-future" ,python-future)
> +       ("python-ipython" ,python-ipython)
> +       ("python-matplotlib" ,python-matplotlib)
> +       ("python-natsort" ,python-natsort)
> +       ("python-numpy" ,python-numpy)
> +       ("python-pandas" ,python-pandas)
> +       ("python-scipy" ,python-scipy)
> +       ("python-six" ,python-six)))
> +    (home-page "http://scikit-bio.org";)
> +    (synopsis "Resources for bioinformatics")
> +    (description
> +     "Scikit-bio (canonically pronounced sigh-kit-buy-oh) is a library for
> +working with biological data in Python, providing data structures, algorithms
> +and educational resources.  It defines python packages for biological
> +sequences, alignments, tree, visualisation, diversity calculation and File
> +I/O.")

Please capitalise “python” throughout and replace “File” with “file”.
I’d also remove “(canonically pronounced sigh-kit-buy-oh)”.

> +    (license bsd-3)
> +    (properties `((python2-variant . ,(delay python2-scikit-bio))))))
> +
> +(define-public python2-scikit-bio
> +  (let ((base (package-with-python2
> +               (strip-python2-variant python-scikit-bio))))
> +    (package
> +      (inherit base)
> +      (native-inputs
> +       (append (package-native-inputs base)
> +               `(("python2-setuptools" ,python2-setuptools)
> +                 ("python2-mock" ,python2-mock)))))))
> +

Looks good to me.

~~ Ricardo