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

Relax minimal numpy version, if possible #40

Closed
kshpytsya opened this issue Dec 5, 2023 · 6 comments
Closed

Relax minimal numpy version, if possible #40

kshpytsya opened this issue Dec 5, 2023 · 6 comments

Comments

@kshpytsya
Copy link

I am currently working on maintaining a legacy codebase which for valid reasons1 requires numpy (>=1.16.0,<1.19.0). I was about to introduce constriction as a mean to significantly reduce memory consumption by some base models, only to find out that I cannot install it due to version conflict.

Is numpy>=1.19.0 a hard requirement?

Footnotes

  1. Old versions of some scientific libraries require old version of numpy. It is impossible to upgrade those libraries because newer versions have introduced incompatible changes to serialized models. We have to support existing models and cannot recreate them.

@robamler
Copy link
Collaborator

robamler commented Dec 12, 2023

Yeah, no need to explain yourself for using old versions of python packages, I know painfully well how disastrous the package management situation is in python :). There's no deep reason for requiring numpy ^1.19.0, it's just the oldest version of numpy I've ever tested with.

I just released a new version of constriction (v0.3.3) that reduces the required numpy version to ^1.16. Please let me know if this resolves your problem (you'll need to use python 3.10 or older; for python 3.11, we require numpy ^1.23.2, and for python 3.12 we require numpy ^1.26.1; relaxing those requirements would be much more difficult).

@kshpytsya
Copy link
Author

Perfect timing. I was about to do a fork. However, there is a problem, probably due to a possible maturin's bug, which causes wheels to contain Requires-Dist: numpy~=1.19 in METADATA regardless of the target python version. A quick scan of maturin's issue tracker didn't discover anything relevant. So I suppose, someone should open an issue. I believe you to be more suitable as my acquaintance with maturin is cursory. If you do not have time for that, I will try opening an issue myself.

P.S. Maybe I am barking at the wrong tree, but I cannot add constriction to my Poetry project requiring Python 3.8 which has other dependencies requiring numpy<1.19.

@kshpytsya
Copy link
Author

P.P.S. It looks like you should change requirements here:

"numpy~=1.19",

See https://www.maturin.rs/metadata.
I have verified that those are the requirements that make it into the wheel metadata, and you have changed only Poetry-specific requirements.

@robamler
Copy link
Collaborator

Sorry, I should have double checked this. Thank you for hunting down the bug yourself!

I fixed it in commit 3f2a96c and just released a new version (this time I did actually look into the wheel's metadata and verified it now contains "numpy~=1.16". Please let me know if this works for you.

@kshpytsya
Copy link
Author

Great! I can confirm that constriction==0.3.4 is installable by Poetry==1.7.1 into Python==3.8.16 along with numpy==1.18.5. I have also verified that encoder works.

I minor detail – I am unsure whether it is worth a separate issue – is the following warning from Poetry:

Warning: Validation of the RECORD file of constriction-0.3.4-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl failed. Please report to the maintainers of that package so they can fix their build process. Details:
In /home/uken/.cache/pypoetry/artifacts/bf/7f/3e/dafe24f40a54f14858cdb770e83c4fc076d2a8e86141bb380a25393718/constriction-0.3.4-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl, LICENSE.html is not mentioned in RECORD

@robamler
Copy link
Collaborator

Great, thanks! I opened #43 for the issue about validating python wheels. The warning is no reason for concern, but I should indeed fix it to prevent the warning message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants