[Bug]: the minimum numpy version is not specified

TomDonoghue commented 2 years ago

What happened?

Because nwbinspector uses numpy.typing, which is new in numpy version 1.20, this makes 1.20 the minimum required numpy version - however this isn't specified in the installation.

Numpy.typing link: https://numpy.org/devdocs/reference/typing.html

Numpy isn't listed in requirements.txt, I think because it is assumed to be installed given that pynwb is required. However, pynwb requires numpy>=1.16, meaning one can validly install pynwb with numpy from 1.16 - 1.19, then install nwbinspector, but then get an import error for numpy.typing.

I think there are two possible easy fixes for this (I'm happy to PR, just not sure which is best):

explicitly list a numpy requirement >= 1.20
keep nwbinspector in line with the numpy requirements of pynwb, and drop using numpy.typing
- numpy.typing is only used in nwbinspector.utils. I think it could be fairly easily dropped to use an older strategy for type checking, and then nwbinspector wouldn't need >=1.20

Steps to Reproduce

Install nwbinspector in an environment with numpy < 1.20, and the following error arises:
`ModuleNotFoundError: No module named 'numpy.typing'`

Traceback

No response

Operating System

macOS

Python Executable

Conda

Python Version

3.8

Usage

Command Line Interface

Were you streaming with ROS3?

No

Package Versions

No response

Code of Conduct

[X] I agree to follow this project's Code of Conduct
[X] Have you ensured this bug was not already reported?
[X] To the best of your ability, have you ensured this is a bug within the code that checks the NWBFile, rather than a bug in the NWBFile reader (e.g., PyNWB or MatNWB)?

CodyCBakerPhD commented 2 years ago

Thanks for submitting an issue @TomDonoghue! There's a PR in progress right now from @jwodder: https://github.com/NeurodataWithoutBorders/nwbinspector/pull/238

But tests appear to be failing there depending on the Python version.

In general, the approach to use numpy.testing annotation typing for ArrayLike arguments is an intended recent addition as we used to manually define much less rigorous implicit Python typing like Union[list, np.ndarray, ...], but this does definitely come with the requirement of using more modern numpy versions.

It can get a tad messy as to what range of version value(s) are supported over which Python versions w.r.t to the bounds set by both HDMF and PyNWB.

A previous fix for this kind of thing on neuroconv: https://github.com/catalystneuro/neuroconv/blob/main/requirements-minimal.txt#L1-L2

Anyway, I'll take a look at this and see what it takes to make the CI happy.

Cheers! Cody Baker, PhD Sr. Neuro-data Scientist at CatalystNeuro

TomDonoghue commented 2 years ago

Ooops - sry, I missed the open PR, as I only looked at the issues!

The fix in #240 looks like it should fix this!

NeurodataWithoutBorders / nwbinspector