Update to Python 3.8 for compatibility with bpaingest master branch

hou098 commented 2 years ago

docker-compose -f docker-compose-build.yml build base dev
ERROR: Package 'bpaingest' requires a different Python: 3.7.12 not in '<4,>=3.8.1'

Tried with Python 3.8:

[hou098@terrible-hf](mailto:hou098@terrible-hf):~/bpaotu$ git diff Dockerfile-base
diff --git a/Dockerfile-base b/Dockerfile-base
index c1e668d..9588114 100644
--- a/Dockerfile-base
+++ b/Dockerfile-base
@@ -1,4 +1,4 @@
-FROM python:3.7-slim
+FROM python:3.8-slim
 LABEL maintainer https://github.com/BioplatformsAustralia/bpaotu

 ENV VIRTUAL_ENV /env
[hou098@terrible-hf](mailto:hou098@terrible-hf):~/bpaotu$

Docker build fails due to https://github.com/BioplatformsAustralia/biom-format/archive/1.1.2bioplatforms1.tar.gz incompatibility with Python 3.8

Running setup.py install for biom-format: finished with status 'error'
  error: subprocess-exited-with-error

  × Running setup.py install for biom-format did not run successfully.
  │ exit code: 1
  ╰─> [296 lines of output]
      /env/lib/python3.8/site-packages/setuptools/dist.py:463: UserWarning: Normalizing '2.1.7-dev' to '2.1.7.dev0'
        warnings.warn(tmpl.format(**locals()))
      running install
      running build
      running build_py
      creating build
      creating build/lib.linux-x86_64-3.8
      creating build/lib.linux-x86_64-3.8/biom
      copying biom/parse.py -> build/lib.linux-x86_64-3.8/biom
      copying biom/__init__.py -> build/lib.linux-x86_64-3.8/biom

...

      copying biom/tests/test_data/test.json.gz -> build/lib.linux-x86_64-3.8/biom/tests/test_data
      running build_ext
      building 'biom._filter' extension
      creating build/temp.linux-x86_64-3.8
      creating build/temp.linux-x86_64-3.8/biom
      gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/env/lib/python3.8/site-packages/numpy/core/include -I/env/lib/python3.8/site-packages/numpy/core/include -I/env/include -I/usr/local/include/python3.8 -c biom/_filter.c -o build/temp.linux-x86_64-3.8/biom/_filter.o
      In file included from /env/lib/python3.8/site-packages/numpy/core/include/numpy/ndarraytypes.h:1948,
                       from /env/lib/python3.8/site-packages/numpy/core/include/numpy/ndarrayobject.h:12,
                       from /env/lib/python3.8/site-packages/numpy/core/include/numpy/arrayobject.h:5,
                       from biom/_filter.c:598:
      /env/lib/python3.8/site-packages/numpy/core/include/numpy/npy_1_7_deprecated_api.h:17:2: warning: #warning "Using deprecated NumPy API, disable it with " "#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-Wcpp]
         17 | #warning "Using deprecated NumPy API, disable it with " \
            |  ^~~~~~~

...

      biom/_filter.c:9201:15: note: in expansion of macro ‘__Pyx_PyCode_New’
       9201 |     py_code = __Pyx_PyCode_New(
            |               ^~~~~~~~~~~~~~~~
      In file included from /usr/local/include/python3.8/compile.h:5,
                       from /usr/local/include/python3.8/Python.h:138,
                       from biom/_filter.c:4:
      /usr/local/include/python3.8/code.h:122:28: note: declared here
        122 | PyAPI_FUNC(PyCodeObject *) PyCode_New(
            |                            ^~~~~~~~~~
      error: command 'gcc' failed with exit status 1
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
error: legacy-install-failure

× Encountered error while trying to install package.
╰─> biom-format

note: This is an issue with the package mentioned above, not pip.
hint: See above for output from the failure.
WARNING: You are using pip version 22.0.4; however, version 22.2.2 is available.
You should consider upgrading via the '/env/bin/python3 -m pip install --upgrade pip' command.
The command '/bin/sh -c pip install --upgrade -r bpaotu/biom-requirements.txt' returned a non-zero code: 1
ERROR: Service 'dev' failed to build : Build failed

On Wed, 2022-09-28 at 02:22 +0000, Mark Tearle wrote:

1st Question – What’s the status of bringing OTU into alignment with bpa-ingest in terms of Python versions? (my > card on Trello is getting me to hunt all the locations where we are using legacy Python versions)

No change. IIRC using a newer python requires updating bpaotu to use https://github.com/BioplatformsAustralia/biom-format It's currently hardwired to https://github.com/BioplatformsAustralia/biom-format/archive/1.1.2bioplatforms1.tar.gz and I don't know if the latest version is compatible

mtearle commented 2 years ago

BPA internal Trello reference - https://trello.com/c/qHRhonTv/1412-update-biom-format-to-upstream

mtearle commented 2 years ago

I've had a look at the fork. There's only one commit different to upstream, and the cython files are not in upstream. Does bpaotu build cleanly if you point to the upstream?

There's nothing obvious in the commit notes to suggest what the need for those files to be committed was.

I can merge the upstream and push it to BioplatformsAustralia/biom-format and tag if required.

hou098 commented 2 years ago

@mtearle

I've had a look at the fork. There's only one commit different to upstream, and the cython files are not in upstream.

https://github.com/BioplatformsAustralia/biom-format says

This branch is 1 commit ahead, 69 commits behind biocore:master.

The C files were removed in https://github.com/biocore/biom-format/commit/a8c179e42010e24d5a0af0b41d432ef42d6d6888

Does bpaotu build cleanly if you point to the upstream?

I'm pretty sure it won't because if we take the .c files out it'll need Cython to build them from .pyx, and there's no Cython in the build environment as far as I can see. Looking at https://github.com/BioplatformsAustralia/biom-format/blob/79b89acde807c4750d072d9608974a8d0ef1ee35/setup.py it seems that Cython is an optional requirement, and defaults to False.

I think what's going on is that back in 2019 those C files needed to be manually regenerated when the Python C API chnaged. The developers probably got jack of that and decided to handle this at install time instead in setup.py

So the options seem to be

Use Cython to regenerate those files for Python 3.8 and tag a new version
Add Cython to to the bpaotu build environment and use https://github.com/biocore/biom-format (some recent tagged release). That's probably the best option and shouldn't be too hard.

mtearle commented 2 years ago

@hou098

Did a quick check across the other BPA repositories, only bpaotu uses biom-format. I think we should go with the second option.

Once we've released the next version of bpaotu to prod built with the other biom-format, I'll archive the BPA repo.

hou098 commented 2 years ago

@mtearle

OK, I will have a go at using https://github.com/biocore/biom-format with Python 3.8.

Once we've released the next version of bpaotu to prod built with the other biom-format, I'll archive the BPA repo.

If it works, I think the best idea might be to merge the current master from https://github.com/biocore/biom-format into https://github.com/BioplatformsAustralia/biom-format in case the original repo disappears for any reason.

hou098 commented 2 years ago

@mtearle

Actually, I see that there is https://pypi.org/project/biom-format/ so I'm just testing

diff --git a/requirements/biom-requirements.txt b/requirements/biom-requirements.txt
index 7e0979b..c8937e7 100644
--- a/requirements/biom-requirements.txt
+++ b/requirements/biom-requirements.txt
@@ -1 +1 @@
-https://github.com/BioplatformsAustralia/biom-format/archive/1.1.2bioplatforms1.tar.gz
+biom-format==2.1.12

Seems to build OK, so if it works OK we can probably just use PyPI and drop the github repo.

hou098 commented 2 years ago

Fixed in e564bc4e99756517d05e80cd6839f3c2d7db8b6f

mtearle commented 2 years ago

I have archived https://github.com/BioplatformsAustralia/biom-format so it is explicit to others that it is not in use any more

BioplatformsAustralia / bpaotu

Update to Python 3.8 for compatibility with bpaingest master branch #242