bug#39872: python-anndata fails to build

[Maxim Cournoyer]

Hello Roel,

Roel Janssen <address@hidden> writes:

> On Mon, 2020-03-02 at 14:29 -0500, Maxim Cournoyer wrote:
>> Build log extract:
>> 
>> --8<---------------cut here---------------start------------->8---
>> starting phase `check'
>> running "python setup.py" with command "test" and parameters ()
>> running test
>> running egg_info
>> writing anndata.egg-info/PKG-INFO
>> writing dependency_links to anndata.egg-info/dependency_links.txt
>> writing requirements to anndata.egg-info/requires.txt
>> writing top-level names to anndata.egg-info/top_level.txt
>> reading manifest file 'anndata.egg-info/SOURCES.txt'
>> reading manifest template 'MANIFEST.in'
>> writing manifest file 'anndata.egg-info/SOURCES.txt'
>> running build_ext
>> anndata (unittest.loader._FailedTest) ... ERROR
>> 
>> ======================================================================
>> ERROR: anndata (unittest.loader._FailedTest)
>> ----------------------------------------------------------------------
>> ImportError: Failed to import test module: anndata
>> Traceback (most recent call last):
>>   File "/gnu/store/78w7y0lxar70j512iqw8x3nimzj10yga-python-
>> 3.7.4/lib/python3.7/unittest/loader.py", line 470, in _find_test_path
>>     package = self._get_module_from_name(name)
>>   File "/gnu/store/78w7y0lxar70j512iqw8x3nimzj10yga-python-
>> 3.7.4/lib/python3.7/unittest/loader.py", line 377, in _get_module_from_name
>>     __import__(name)
>>   File "/tmp/guix-build-python-anndata-0.6.18.drv-0/anndata-
>> 0.6.18/anndata/__init__.py", line 1, in <module>
>>     from .base import AnnData
>>   File "/tmp/guix-build-python-anndata-0.6.18.drv-0/anndata-
>> 0.6.18/anndata/base.py", line 21, in <module>
>>     from scipy.sparse.sputils import IndexMixin
>> ImportError: cannot import name 'IndexMixin' from 'scipy.sparse.sputils'
>> (/gnu/store/fd99znbw3w7k05r8xkm03w1kvpa6yl1w-python-scipy-
>> 1.3.2/lib/python3.7/site-packages/scipy/sparse/sputils.py)
>> 
>> 
>> ----------------------------------------------------------------------
>> Ran 1 test in 0.000s
>> 
>> FAILED (errors=1)
>> Test failed: <unittest.runner.TextTestResult run=1 errors=1 failures=0>
>> error: Test failed: <unittest.runner.TextTestResult run=1 errors=1 
>> failures=0>
>> command "python" "-c" "import setuptools,
>> tokenize;__file__='setup.py';f=getattr(tokenize, 'open',
>> open)(__file__);code=f.read().replace('\\r\\n',
>> '\\n');f.close();exec(compile(code, __file__, 'exec'))" "test" failed with
>> status 1
>> builder for `/gnu/store/ma6q97h3245r7sxhqms6q1csbkss2wvq-python-anndata-
>> 0.6.18.drv' failed with exit code 1
>> build of 
>> /gnu/store/ma6q97h3245r7sxhqms6q1csbkss2wvq-python-anndata-0.6.18.drv 
>> failed
>> View build log at '/var/log/guix/drvs/ma/6q97h3245r7sxhqms6q1csbkss2wvq-
>> python-anndata-0.6.18.drv.bz2'.
>> guix build: error: build of
>> `/gnu/store/ma6q97h3245r7sxhqms6q1csbkss2wvq-python-anndata-0.6.18.drv'
>> failed
>> --8<---------------cut here---------------end--------------->8---
>> 
>> On master c1febbbf94ee794d7a97dbde12102634f2b13529.

The problem has since been fixed on master with commits
7032c1cc6040a14e482e71b0cfbdedf957da4f8a,
8e04b233f130e01b5b6a41dfcdeb5e622d43f751,
241409cd8afe45fdecfbd7d24cdeb08aa2a43884 and
a098b3498052e046c8338e0630e0166bcb12355d; Ricardo probably did not
notice about this bug report and your posted patches.

> I sent an e-mail from another address that doesn't seem to have made the guix-
> patches mailing list.  The attached patches updates the version of python-
> anndata, and fixes the build.
>
> Note that for python-zarr I disabled the tests because the majority of the 
> tests
> need external packages.  We could add all dependencies to make the test suite
> work, but that requires a significant amount of work (for example, the Azure 
> SDK
> for Python is one of the external dependencies).
>
> Is it OK to move forward with the disabled test suite for python-zarr?

I think it would have been acceptable (given that anndata was broken and
needed a fix); but enabling the tests that can be run ideally should be
attempted.  See in commit 7032c1cc6040a14e482e71b0cfbdedf957da4f8a how
this was achieved.

You'll find a summary review of your patches below.  I've made sure that
what got committed was not missing anything.

> From 08e25e127b7c19b339c5257d61b6d2c1b9d9a2c9 Mon Sep 17 00:00:00 2001
> From: Roel Janssen <address@hidden>
> Date: Wed, 4 Mar 2020 11:13:21 +0100
> Subject: [PATCH 4/4] gnu: Update python-anndata to 0.7.1.
>
> * gnu/packages/python-xyz.scm (python-anndata): Update to 0.7.1; Add
>   dependencies to run the test suite.
> ---
>  gnu/packages/python-xyz.scm | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
> index 3eabc9e3fa..2ec79404f2 100644
> --- a/gnu/packages/python-xyz.scm
> +++ b/gnu/packages/python-xyz.scm
> @@ -15800,15 +15800,22 @@ compressed, N-dimensional arrays for Python.")
>  (define-public python-anndata
>    (package
>      (name "python-anndata")
> -    (version "0.6.18")
> +    (version "0.7.1")
>      (source
>       (origin
>         (method url-fetch)
>         (uri (pypi-uri "anndata" version))
>         (sha256
>          (base32
> -         "03x83yjaccbqszj7x4fwwmpil0ai59yx64d1zmf2691za3j03w73"))))
> +         "0rnfbpr55j1a1bi2kd4mz444741hrn74kz90h5rnjr59jmpfnh09"))))
>      (build-system python-build-system)
> +    ;; The following inputs are needed for the test suite.
> +    (native-inputs
> +     `(("python-setuptools-scm" ,python-setuptools-scm)
> +       ("python-packaging" ,python-packaging)
> +       ("python-importlib-metadata" ,python-importlib-metadata)
> +       ("python-numcodecs" ,python-numcodecs)
> +       ("python-zarr" ,python-zarr)))
>      (propagated-inputs
>       `(("python-h5py" ,python-h5py)
>         ("python-natsort" ,python-natsort)
> -- 
> 2.25.1

Here the default check phase (which does 'python setup.py test'), was
not running the test suite.  It had to be overriden by a 'pytest'
invocation.  I find this to be very common in Python packages; I think
there was something about deprecating 'python setup.py test' to; maybe
we should look into making this phase of the python-build-system smarter.

> From 8a01cc8c797dde47d6e4982f7b8710fc9bf106e1 Mon Sep 17 00:00:00 2001
> From: Roel Janssen <address@hidden>
> Date: Wed, 4 Mar 2020 11:12:41 +0100
> Subject: [PATCH 3/4] gnu: Add python-zarr.
>
> * gnu/packages/python-xyz.scm (python-zarr): New variable.
> ---
>  gnu/packages/python-xyz.scm | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
> index 067756cae8..3eabc9e3fa 100644
> --- a/gnu/packages/python-xyz.scm
> +++ b/gnu/packages/python-xyz.scm
> @@ -15771,6 +15771,32 @@ codecs for use in data storage and communication 
> applications.")
>      (description "Draws ASCII trees.")
>      (license license:expat)))
>  
> +(define-public python-zarr
> +  (package
> +    (name "python-zarr")
> +    (version "2.4.0")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (pypi-uri "zarr" version))
> +              (sha256
> +               (base32
> +                "026n3sjzjv2gmwx6y72b8ij0hk42bc8zdbvfj5gdqzd4i6wj3ajk"))))
> +    (build-system python-build-system)
> +    (arguments
> +     ;; The tests import h5py, lmdb, pymongo, bsddb3, and azure.storage.blob 
> from
> +     ;; the Azure SDK for python.
> +     `(#:tests? #f))
> +    (propagated-inputs
> +     `(("python-asciitree" ,python-asciitree)
> +       ("python-fasteners" ,python-fasteners)
> +       ("python-numcodecs" ,python-numcodecs)
> +       ("python-numpy" ,python-numpy)))
> +    (home-page "https://github.com/zarr-developers/zarr-python";)
> +    (synopsis "Implementation of chunked, compressed, N-dimensional arrays 
> for Python.")
> +    (description "This package provides an implementation of chunked,
> +compressed, N-dimensional arrays for Python.")
> +    (license license:expat)))
> +
>  (define-public python-anndata
>    (package
>      (name "python-anndata")
> -- 
> 2.25.1

LGTM, but see commit for a way to partially enable the test suite:
7032c1cc6040a14e482e71b0cfbdedf957da4f8a.

> From f48903066a6987c3af2b4130dde03ee2c8c9b70e Mon Sep 17 00:00:00 2001
> From: Roel Janssen <address@hidden>
> Date: Wed, 4 Mar 2020 11:12:09 +0100
> Subject: [PATCH 2/4] gnu: Add python-asciitree.
>
> * gnu/packages/python-xyz.scm (python-asciitree): New variable.
> ---
>  gnu/packages/python-xyz.scm | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
> index dec41a8c5e..067756cae8 100644
> --- a/gnu/packages/python-xyz.scm
> +++ b/gnu/packages/python-xyz.scm
> @@ -15755,6 +15755,22 @@ tool).")
>  codecs for use in data storage and communication applications.")
>      (license license:expat)))
>  
> +(define-public python-asciitree
> +  (package
> +    (name "python-asciitree")
> +    (version "0.3.3")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (pypi-uri "asciitree" version))
> +              (sha256
> +               (base32
> +                "0vhgri2m2xlnibhz4xwn4hpbc7xacisxjqrk6k5kyppq96vbk92a"))))
> +    (build-system python-build-system)
> +    (home-page "http://github.com/mbr/asciitree";)

The URL should use HTTPS.

> +    (synopsis "Draws ASCII trees.")
> +    (description "Draws ASCII trees.")
> +    (license license:expat)))

The description should be expound a bit.

>  (define-public python-anndata
>    (package
>      (name "python-anndata")
> -- 
> 2.25.1
>
>
> From 0fc3b77486289835f28f16a6380c15fd128cab6b Mon Sep 17 00:00:00 2001
> From: Roel Janssen <address@hidden>
> Date: Wed, 4 Mar 2020 11:11:17 +0100
> Subject: [PATCH 1/4] gnu: Add python-numcodecs.
>
> * gnu/packages/python-xyz.scm (python-numcodecs): New variable.
> ---
>  gnu/packages/python-xyz.scm | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
> index f3519b75f6..dec41a8c5e 100644
> --- a/gnu/packages/python-xyz.scm
> +++ b/gnu/packages/python-xyz.scm
> @@ -40,7 +40,7 @@
>  ;;; Copyright © 2017 Ben Sturmfels <address@hidden>
>  ;;; Copyright © 2017, 2018, 2019 Mathieu Othacehe <address@hidden>
>  ;;; Copyright © 2017 José Miguel Sánchez García <address@hidden>
> -;;; Copyright © 2017 Roel Janssen <address@hidden>
> +;;; Copyright © 2017 2020 Roel Janssen <address@hidden>
>  ;;; Copyright © 2017, 2018, 2019 Kei Kebreau <address@hidden>
>  ;;; Copyright © 2017 Rutger Helling <address@hidden>
>  ;;; Copyright © 2017 Muriithi Frederick Muriuki <address@hidden>
> @@ -15733,6 +15733,28 @@ infrastructure at import time, runtime, or 
> statically (using the included pycc
>  tool).")
>      (license license:bsd-3)))
>  
> +(define-public python-numcodecs
> +  (package
> +    (name "python-numcodecs")
> +    (version "0.6.4")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (pypi-uri "numcodecs" version))
> +              (sha256
> +               (base32
> +                "0kbfr8pl3x9glsypbq8hzim003f16ml1b1cvgrh4w1sdvgal6j7g"))))
> +    (build-system python-build-system)
> +    (propagated-inputs
> +     `(("python-numpy" ,python-numpy)
> +       ("python-setuptools-scm" ,python-setuptools-scm)
> +       ("python-msgpack" ,python-msgpack)
> +       ("python-pytest" ,python-pytest)))

Pytest and setuptools plugins are only required at build time, hence
would be better found in 'native-inputs'.

> +    (home-page "https://github.com/zarr-developers/numcodecs";)
> +    (synopsis "Buffer compression and transformation codecs")
> +    (description "This package provides buffer compression and transformation
> +codecs for use in data storage and communication applications.")
> +    (license license:expat)))

Otherwise LGTM!

I'm sorry that this work was duplicated; I hope this small review can
compensate for it a little :-).

Thank you; I'm closing the issue.

Maxim