Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict numpy version #597

Closed
wants to merge 2 commits into from
Closed

Restrict numpy version #597

wants to merge 2 commits into from

Conversation

jwodder
Copy link
Member

@jwodder jwodder commented Apr 27, 2021

A recent version of hdmf added support for newer versions of numpy than 1.19, which is the latest version that version 2 of h5py can be built with. As a result, a newer version of numpy was being installed than h5py could handle. This patch restricts the numpy version on Python 3.9+, for which h5py 2.x does not provide wheels. It should become unnecessary once hmdf supports h5py 3.0.

@jwodder jwodder added the dependencies Update one or more dependencies version label Apr 27, 2021
@codecov
Copy link

codecov bot commented Apr 27, 2021

Codecov Report

Merging #597 (77d8a50) into master (242c5af) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #597   +/-   ##
=======================================
  Coverage   81.26%   81.26%           
=======================================
  Files          62       62           
  Lines        6651     6651           
=======================================
  Hits         5405     5405           
  Misses       1246     1246           
Flag Coverage Δ
unittests 81.26% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 242c5af...77d8a50. Read the comment docs.

@yarikoptic
Copy link
Member

should then we remove

.github/workflows/test.yml:        pip install 'numpy<1.19.4'

and users would be as happy in installing dandi on py 3.9 as on any other?

@jwodder jwodder changed the title Restrict numpy version for Python 3.9 Restrict numpy version Apr 28, 2021
@jwodder
Copy link
Member Author

jwodder commented Apr 28, 2021

@yarikoptic No, that still won't work, though testing that out did reveal that the numpy version restriction should be applied to all Python versions, not just 3.9.

@yarikoptic
Copy link
Member

Why (please provide explanation on that commit message details)? Insofar afaik it was ok without that.

@jwodder
Copy link
Member Author

jwodder commented Apr 28, 2021

@yarikoptic I believe we need to restrict numpy for all Python versions because of recent versions of hdmf supporting versions of numpy newer than 1.19.3. h5py 2.10.0 requires a pre-1.20 version of numpy in order to run, but because it was released before that numpy version, they didn't know they had to declare that, and so h5py 2.10.0 does not sufficiently restrict the numpy version. When installing dandi without this PR, you end up getting the latest version of numpy, version 1.20.2, which is incompatible with h5py 2.10.0.

I originally thought that this PR's change was only needed for Python 3.9 because that is the only Python version supported by dandi for which h5py does not provide wheels, i.e., the only version for which h5py needs to be built from source. Also, because this was the only Python version that was previously giving us depedency mismatch issues.

Our tests weren't catching the fact that this version restriction was needed because they explicitly installed numpy 1.19.3 under all Python versions.

@yarikoptic
Copy link
Member

well, in devel envs with older python I think I never needed it, but I most often created them with --system-site-packages so likely used system (older) wide numpy. But users did not raise issues either.

So I did fresh virtualenv with python 3.7.3 and pip install dandi
$> py=3; d=venvs/dev$py; virtualenv --python=python$py  $d && source $d/bin/activate && pip3 install dandi 
Already using interpreter /usr/bin/python3
Using base prefix '/usr'
New python executable in /tmp/venvs/dev3/bin/python3
Also creating executable in /tmp/venvs/dev3/bin/python
Installing setuptools, pkg_resources, pip, wheel...done.
Collecting dandi
  Using cached dandi-0.14.2-py3-none-any.whl (173 kB)
Collecting semantic-version
  Using cached semantic_version-2.8.5-py2.py3-none-any.whl (15 kB)
Collecting jsonschema
  Using cached jsonschema-3.2.0-py2.py3-none-any.whl (56 kB)
Collecting fasteners
  Using cached fasteners-0.16-py2.py3-none-any.whl (28 kB)
Collecting requests~=2.20
  Using cached requests-2.25.1-py2.py3-none-any.whl (61 kB)
Collecting pydantic>=1.8.1
  Using cached pydantic-1.8.1-cp37-cp37m-manylinux2014_x86_64.whl (10.1 MB)
Collecting pynwb!=1.1.0,>=1.0.3
  Downloading pynwb-1.4.0-py2.py3-none-any.whl (94 kB)
     |████████████████████████████████| 94 kB 99 kB/s 
Collecting email-validator
  Using cached email_validator-1.1.2-py2.py3-none-any.whl (17 kB)
Collecting fscacher
  Using cached fscacher-0.1.4-py3-none-any.whl (9.7 kB)
Collecting tqdm
  Downloading tqdm-4.60.0-py2.py3-none-any.whl (75 kB)
     |████████████████████████████████| 75 kB 285 kB/s 
Collecting python-dateutil
  Using cached python_dateutil-2.8.1-py2.py3-none-any.whl (227 kB)
Collecting keyrings.alt
  Downloading keyrings.alt-4.0.2-py3-none-any.whl (20 kB)
Collecting click-didyoumean
  Using cached click-didyoumean-0.0.3.tar.gz (2.6 kB)
Collecting tenacity
  Using cached tenacity-7.0.0-py2.py3-none-any.whl (23 kB)
Collecting pycryptodomex
  Downloading pycryptodomex-3.10.1-cp35-abi3-manylinux2010_x86_64.whl (1.9 MB)
     |████████████████████████████████| 1.9 MB 7.9 MB/s 
Collecting appdirs
  Using cached appdirs-1.4.4-py2.py3-none-any.whl (9.6 kB)
Collecting girder-client
  Downloading girder-client-3.1.4.tar.gz (20 kB)
Collecting joblib
  Downloading joblib-1.0.1-py3-none-any.whl (303 kB)
     |████████████████████████████████| 303 kB 24.4 MB/s 
Collecting humanize
  Downloading humanize-3.4.1-py3-none-any.whl (73 kB)
     |████████████████████████████████| 73 kB 136 kB/s 
Collecting keyring
  Downloading keyring-23.0.1-py3-none-any.whl (33 kB)
Collecting click
  Using cached click-7.1.2-py2.py3-none-any.whl (82 kB)
Collecting ruamel.yaml<1,>=0.15
  Downloading ruamel.yaml-0.17.4-py3-none-any.whl (101 kB)
     |████████████████████████████████| 101 kB 316 kB/s 
Collecting typing-extensions
  Using cached typing_extensions-3.7.4.3-py3-none-any.whl (22 kB)
Collecting importlib-metadata
  Downloading importlib_metadata-4.0.1-py3-none-any.whl (16 kB)
Collecting pyout!=0.6.0,>=0.5
  Using cached pyout-0.7.0-py3-none-any.whl (51 kB)
Collecting etelemetry>=0.2.0
  Using cached etelemetry-0.2.2-py3-none-any.whl (6.2 kB)
Collecting ci-info>=0.2
  Using cached ci_info-0.2.0-py3-none-any.whl (6.9 kB)
Collecting pandas>=0.23
  Downloading pandas-1.2.4-cp37-cp37m-manylinux1_x86_64.whl (9.9 MB)
     |████████████████████████████████| 9.9 MB 21.9 MB/s 
Collecting numpy>=1.16
  Downloading numpy-1.20.2-cp37-cp37m-manylinux2010_x86_64.whl (15.3 MB)
     |████████████████████████████████| 15.3 MB 29.1 MB/s 
Collecting h5py>=2.9
  Downloading h5py-3.2.1-cp37-cp37m-manylinux1_x86_64.whl (4.1 MB)
     |████████████████████████████████| 4.1 MB 41.3 MB/s 
Collecting hdmf<3,>=2.1.0
  Downloading hdmf-2.5.1-py2.py3-none-any.whl (163 kB)
     |████████████████████████████████| 163 kB 49.1 MB/s 
Collecting cached-property
  Downloading cached_property-1.5.2-py2.py3-none-any.whl (7.6 kB)
Collecting scipy<2,>=1.1
  Downloading scipy-1.6.3-cp37-cp37m-manylinux1_x86_64.whl (27.4 MB)
     |████████████████████████████████| 27.4 MB 41 kB/s 
Collecting h5py>=2.9
  Using cached h5py-2.10.0-cp37-cp37m-manylinux1_x86_64.whl (2.9 MB)
Collecting six
  Downloading six-1.15.0-py2.py3-none-any.whl (10 kB)
Requirement already satisfied: setuptools in ./venvs/dev3/lib/python3.7/site-packages (from jsonschema->dandi) (56.0.0)
Collecting pyrsistent>=0.14.0
  Downloading pyrsistent-0.17.3.tar.gz (106 kB)
     |████████████████████████████████| 106 kB 79.4 MB/s 
Collecting attrs>=17.4.0
  Downloading attrs-20.3.0-py2.py3-none-any.whl (49 kB)
     |████████████████████████████████| 49 kB 11 kB/s 
Collecting pytz>=2017.3
  Downloading pytz-2021.1-py2.py3-none-any.whl (510 kB)
     |████████████████████████████████| 510 kB 35.2 MB/s 
Collecting blessings
  Using cached blessings-1.7-py3-none-any.whl (18 kB)
Collecting urllib3<1.27,>=1.21.1
  Using cached urllib3-1.26.4-py2.py3-none-any.whl (153 kB)
Collecting certifi>=2017.4.17
  Downloading certifi-2020.12.5-py2.py3-none-any.whl (147 kB)
     |████████████████████████████████| 147 kB 37.6 MB/s 
Collecting idna<3,>=2.5
  Using cached idna-2.10-py2.py3-none-any.whl (58 kB)
Collecting chardet<5,>=3.0.2
  Using cached chardet-4.0.0-py2.py3-none-any.whl (178 kB)
Collecting ruamel.yaml.clib>=0.1.2
  Downloading ruamel.yaml.clib-0.2.2-cp37-cp37m-manylinux1_x86_64.whl (547 kB)
     |████████████████████████████████| 547 kB 42.8 MB/s 
Collecting dnspython>=1.15.0
  Downloading dnspython-2.1.0-py3-none-any.whl (241 kB)
     |████████████████████████████████| 241 kB 31.7 MB/s 
Collecting diskcache
  Downloading diskcache-5.2.1-py3-none-any.whl (44 kB)
     |████████████████████████████████| 44 kB 93 kB/s 
Collecting requests_toolbelt
  Using cached requests_toolbelt-0.9.1-py2.py3-none-any.whl (54 kB)
Collecting zipp>=0.5
  Downloading zipp-3.4.1-py3-none-any.whl (5.2 kB)
Collecting SecretStorage>=3.2
  Downloading SecretStorage-3.3.1-py3-none-any.whl (15 kB)
Collecting jeepney>=0.4.2
  Downloading jeepney-0.6.0-py3-none-any.whl (45 kB)
     |████████████████████████████████| 45 kB 102 kB/s 
Collecting cryptography>=2.0
  Downloading cryptography-3.4.7-cp36-abi3-manylinux2014_x86_64.whl (3.2 MB)
     |████████████████████████████████| 3.2 MB 32.7 MB/s 
Collecting cffi>=1.12
  Using cached cffi-1.14.5-cp37-cp37m-manylinux1_x86_64.whl (402 kB)
Collecting pycparser
  Using cached pycparser-2.20-py2.py3-none-any.whl (112 kB)
Building wheels for collected packages: pyrsistent, click-didyoumean, girder-client
  Building wheel for pyrsistent (setup.py) ... done
  Created wheel for pyrsistent: filename=pyrsistent-0.17.3-cp37-cp37m-linux_x86_64.whl size=104232 sha256=9101c45d5ff0ae9e9ed05792b5a852739f574e44ecee89af9cecd14501616257
  Stored in directory: /home/yoh/.cache/pip/wheels/a5/52/bf/71258a1d7b3c8cbe1ee53f9314c6f65f20385481eaee573cc5
  Building wheel for click-didyoumean (setup.py) ... done
  Created wheel for click-didyoumean: filename=click_didyoumean-0.0.3-py3-none-any.whl size=2146 sha256=5f53933707fa506fbe85753b595ce10a49233901ea722d89d2c7ff732ea2633c
  Stored in directory: /home/yoh/.cache/pip/wheels/fa/7d/7c/7870b469b2f29dd833cf4ce37e5e7ab32d3488d405014232f6
  Building wheel for girder-client (setup.py) ... done
  Created wheel for girder-client: filename=girder_client-3.1.4-py3-none-any.whl size=21182 sha256=e0e9ed97fa86a96c1a8b7e1bcdc86680aaa93501c5fb6c4829fdea8e9d51a2a2
  Stored in directory: /home/yoh/.cache/pip/wheels/0e/9f/9c/bb3198f577b8da5c4f0a32a686fb46c74fbb333d7bf09ceb5e
Successfully built pyrsistent click-didyoumean girder-client
Installing collected packages: zipp, typing-extensions, six, pycparser, urllib3, ruamel.yaml.clib, pytz, python-dateutil, pyrsistent, numpy, importlib-metadata, idna, chardet, cffi, certifi, attrs, scipy, ruamel.yaml, requests, pandas, jsonschema, jeepney, h5py, cryptography, SecretStorage, requests-toolbelt, joblib, hdmf, dnspython, diskcache, click, ci-info, blessings, appdirs, tqdm, tenacity, semantic-version, pyout, pynwb, pydantic, pycryptodomex, keyrings.alt, keyring, humanize, girder-client, fscacher, fasteners, etelemetry, email-validator, click-didyoumean, dandi
Successfully installed SecretStorage-3.3.1 appdirs-1.4.4 attrs-20.3.0 blessings-1.7 certifi-2020.12.5 cffi-1.14.5 chardet-4.0.0 ci-info-0.2.0 click-7.1.2 click-didyoumean-0.0.3 cryptography-3.4.7 dandi-0.14.2 diskcache-5.2.1 dnspython-2.1.0 email-validator-1.1.2 etelemetry-0.2.2 fasteners-0.16 fscacher-0.1.4 girder-client-3.1.4 h5py-2.10.0 hdmf-2.5.1 humanize-3.4.1 idna-2.10 importlib-metadata-4.0.1 jeepney-0.6.0 joblib-1.0.1 jsonschema-3.2.0 keyring-23.0.1 keyrings.alt-4.0.2 numpy-1.20.2 pandas-1.2.4 pycparser-2.20 pycryptodomex-3.10.1 pydantic-1.8.1 pynwb-1.4.0 pyout-0.7.0 pyrsistent-0.17.3 python-dateutil-2.8.1 pytz-2021.1 requests-2.25.1 requests-toolbelt-0.9.1 ruamel.yaml-0.17.4 ruamel.yaml.clib-0.2.2 scipy-1.6.3 semantic-version-2.8.5 six-1.15.0 tenacity-7.0.0 tqdm-4.60.0 typing-extensions-3.7.4.3 urllib3-1.26.4 zipp-3.4.1
pip3 install dandi  17.24s user 4.73s system 40% cpu 54.607 total

it did get numpy-1.20.2-cp37-cp37m-manylinux2010_x86_64.whl and h5py-3.2.1-cp37-cp37m-manylinux1_x86_64.whl but dandi seems to function ok. even dandi ls on some random nwb file worked.... so I did python -m pytest -s -v --pyargs dandi and 246 passed, 5 skipped, 7 xfailed, 160 warnings. So overall it seems we might be Ok even without such extra restrictions for earlier versions? I would prefer to not introduce some unless we know it is actually needed

@jwodder
Copy link
Member Author

jwodder commented Apr 28, 2021

@yarikoptic Well, now I'm just confused, because when I omit the line from this PR on my Mac and run tox in a fresh checkout, I get an error when h5py is loaded, for both Python 3.8 and 3.9.

@jwodder
Copy link
Member Author

jwodder commented Apr 28, 2021

@yarikoptic I tried running tox on dandi master in a python:3.9 Docker container, and there was no dependency problem (There were some new errors involving sample NWBs, though). I then purged my pip cache outside of Docker and reran tox, and I could not reproduce the failure I was seeing earlier. However, do note that, when I tried deleting the pip install 'numpy<1.19.4' line from test.yml a few hours ago, all of the tests failed with dependency errors.

@yarikoptic
Copy link
Member

re the one which failed after removing pre-installation: Well, I guess it all boil down to still suboptimal dependencies resolution within pip, which might still not be enforcing some versioning requirements. if you have a record of that run you might see what numpy was installed and what pip said.
Overall - since seems to not really resolve the existing issue, still reluctant to restrict and just cause pip to just ignore it anyways ;-)

@jwodder
Copy link
Member Author

jwodder commented Apr 28, 2021

@yarikoptic All of the new test failures appear to be due to h5py 2.10 doing things with numpy that were deprecated in numpy 1.20. h5py has fixed this in the 3.x branch, but, alas, hdmf still requires h5py < 3.

@yarikoptic
Copy link
Member

rright... it is just the odd pip -- it downloads both versions of h5py and I reacted to the first one in

Collecting h5py>=2.9
  Downloading h5py-3.2.1-cp37-cp37m-manylinux1_x86_64.whl (4.1 MB)
     |████████████████████████████████| 4.1 MB 41.3 MB/s 
Collecting hdmf<3,>=2.1.0
  Downloading hdmf-2.5.1-py2.py3-none-any.whl (163 kB)
     |████████████████████████████████| 163 kB 49.1 MB/s 
Collecting cached-property
  Downloading cached_property-1.5.2-py2.py3-none-any.whl (7.6 kB)
Collecting scipy<2,>=1.1
  Downloading scipy-1.6.3-cp37-cp37m-manylinux1_x86_64.whl (27.4 MB)
     |████████████████████████████████| 27.4 MB 41 kB/s 
Collecting h5py>=2.9
  Using cached h5py-2.10.0-cp37-cp37m-manylinux1_x86_64.whl (2.9 MB)

so it did install h5py 2.10.0 at the end for me. but it did install numpy-1.20.2 and things are fine. So -- I guess the situation is just a mess and some whl's for some python versions on some platforms are missing or broken/not interoperable, but it is not generally so.
And those requirements for hdmf should and are set by it (it is used by setup.py and == is replaced with >= for install_requires):

(git)lena:~exppsy/hdmf-upstream[tags/2.5.1^0]git
$> cat requirements-min.txt       
# package dependencies and their minimum versions for installing HDMF
# the requirements here specify '==' for testing; setup.py replaces '==' with '>='
h5py==2.9,<3  # support for setting attrs to lists of utf-8 added in 2.9
numpy==1.16,<1.21

so if pip manages to ignore them or breaks, I do not think we should be messing with our list of requirements to somehow try to mediate it on dandi-cli level.

@jwodder
Copy link
Member Author

jwodder commented Apr 29, 2021

@yarikoptic After some experimentation, I've found that this PR can be ignored, and the pip install 'numpy<1.19.4' line can be removed from test.yml, as long as pytest is configured to ignore a couple more warnings. See #603. I have no idea what the warning experience will be like for end-users running dandi with numpy 1.20, though.

@yarikoptic
Copy link
Member

cool. So let's indeed just close this one for now . thanks for all the digging!

@yarikoptic yarikoptic closed this Apr 29, 2021
@jwodder jwodder deleted the numpy-restrict branch October 11, 2021 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Update one or more dependencies version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants