NeurodataWithoutBorders / pynwb

A Python API for working with Neurodata stored in the NWB Format
https://pynwb.readthedocs.io
Other
176 stars 85 forks source link

support h5py 3.x #1319

Closed satra closed 1 year ago

satra commented 3 years ago

Briefly describe the needed feature as well as the reasoning behind it

Is your feature request related to a problem? Please describe. currently, requirements.txt forces h5py to 2.10. so every package is forced to use this version. with the emergence of h5py 3.x makes it harder to install both pynwb and any 3.x dependent package in the same environment.

Describe the solution you'd like support 3.x, and more generally don't restrict h5py to a specific version.

Describe alternatives you've considered multiple environments, but that's not always super helpful

Additional context

also the ros3 patch i submitted to h5py was for 3.x, and conda-forge should soon support the ros3 support across OS'es. so it would be nice if all of these things worked together. thus filing this issue at least as a placeholder for upgrade.

Checklist

cc: @yarikoptic

rly commented 3 years ago

Adding support for h5py 3.x in HDMF is being tracked here: https://github.com/hdmf-dev/hdmf/issues/458 . I believe there are no h5py specific features in PyNWB proper.

Regarding requirements files, note that we are following the convention where requirements.txt specifies pinned requirements for a reproducible environment and setup.py specifies minimum requirements, e.g., h5py>=2.9.

Whereas install_requires requirements are minimal, requirements files often contain an exhaustive listing of pinned versions for the purpose of achieving repeatable installations of a complete environment. https://packaging.python.org/discussions/install-requires-vs-requirements/

Until h5py 3.x is supported, we should update install_requires of setup.py to block h5py>=3.0.

satra commented 3 years ago

@rly i was looking at this line: https://github.com/NeurodataWithoutBorders/pynwb/blob/dev/requirements.txt#L1

all of the lines there specify very specific versions.

rly commented 3 years ago

@satra From my understanding of the Python packaging convention, that file should pin very specific versions so that users/developers can recreate a known working environment. Most users will not use requirements.txt, and instead use pip install which uses the minimum requirements specified in setup.py (which compute to, e.g., h5py>=2.9).

(As an aside, we should update requirements.txt to include pinned versions of all sub-dependencies using pip freeze.)

satra commented 3 years ago

@rly - that depends on how people are installing. one can also do pip install -r requirements.txt but yes, if they simply do a pip install it will use the constraints in setup.py.

satra commented 3 years ago

@rly - also this https://github.com/hdmf-dev/hdmf/blob/dev/requirements-min.txt#L3 , which is used in the hdmf setup.py and that pins things as well.

rly commented 3 years ago

@satra if a user installs using pip install -r requirements.txt, we assume that they want to reproduce a known working environment exactly as pinned by that file -- see use 1 here https://pip.pypa.io/en/stable/user_guide/#requirements-files which seems like the most common use case from my readings. If a user wants to use different versions of the requirements, then they should not use the above command or they should upgrade the requirements after running the above command.

hdmf setup.py converts h5py==2.9,<3 from https://github.com/hdmf-dev/hdmf/blob/dev/requirements-min.txt#L3 to h5py>=2.9,<3.

satra commented 3 years ago

ah i hadn't seen the code closely enough.

bendichter commented 1 year ago

@oruebel , @rly this can be closed, right?

rly commented 1 year ago

Yes