search

OpenCyphal / pycyphal

Python implementation of the Cyphal protocol stack.
https://pycyphal.readthedocs.io/
MIT License
119 stars 106 forks source link

Numpy ~=1.19 requirement is unnecessarily restrictive #142

Closed alkasm closed 3 years ago

alkasm commented 3 years ago

The numpy version requirement was bumped from ~=1.16 to ~=1.19 to support the bitorder kwarg in np.packbits() in https://github.com/UAVCAN/pyuavcan/pull/124/commits/e24d2ce3665b03eb4638c97c3965a48bd98b4e91, although this kwarg was introduced in numpy 1.17 (the release notes incorrectly calls the kwarg order but the function docs specify it correctly).

Not really a big issue but if there isn't a specific need for 1.19, there's no reason for this library to require it.

pavel-kirienko commented 3 years ago

Does this requirement create any specific issues in your case? If not, I am inclined to keep it as-is to reduce the variability of production environments.

alkasm commented 3 years ago

Not any unworkable one, I can patch the requirement for example in my case. I felt it kept with the ethos of minimal dependencies to also require the minimum viable version. Since numpy changed some of its internals to be able to use 64-bit BLAS and LAPACK libs in 1.18 and removed support for Python < 3.6 in 1.19, it could be limiting for certain toolchains. OTOH I couldn't find any documentation for PyUAVCAN specifying the minimal Python version targeted, so Python 3.5 and lower support might not be interesting to you.

pavel-kirienko commented 3 years ago

The minimal required version is not explicitly documented anywhere but we enforce this:

https://github.com/UAVCAN/pyuavcan/blob/8bc9c4b936b36642a6da3723c4013eb367c29464/pyuavcan/__init__.py#L62-L63

If you prefer to lower the minimal version requirement down to 1.18 or 1.17, a pull request would be accepted.