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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upper bound version constraint on Numpy? #548

Closed
nollety opened this issue Feb 16, 2023 · 1 comment 路 Fixed by #569
Closed

Upper bound version constraint on Numpy? #548

nollety opened this issue Feb 16, 2023 · 1 comment 路 Fixed by #569
Labels
good first issue Good for newcomers
Milestone

Comments

@nollety
Copy link
Contributor

nollety commented Feb 16, 2023

I was wondering why does radis sets an upper bound version constraint on Numpy in its setup file?

馃悰 Describe the issue

When adding radis to a Python environment where a version of Numpy larger than 1.22.3 is required (either from the current project, or from other dependencies), the package manager is going to run into a resolution error as radis requires numpy<=1.22.3.

馃幆 Expected behavior

Usually, it is not recommended to have upper bound version constraint for dependencies, but I don't know there are specific reasons here to avoid Numpy versions larger than 1.22.3?

馃挕 Possible solutions

Unless radis is incompatible with numpy>1.22.3, we could simply update the constraint set by this line in the setup.py file (similarly in the development conda environment file) to:

"numpy>=1.22.3"

(perhaps a typo changed > into <?)

If I change numpy>=1.22.3 to numpy, in environment.yml and let conda determine what version of Numpy to install, it picks 1.23.5 (latest version at the time of writing is 1.24.2). If I run the tests with this change, I have all tests passing.


Tip : you can vote for New Features on https://feathub.com/radis/radis
Also check for the Long Term Todo List

@erwanp
Copy link
Member

erwanp commented Feb 16, 2023

This was due to an old incompatibility with Vaex few months ago #490
In october we still didn't have confirmation of compatibility with vaex vaexio/vaex#2062 ; I just asked there for an update.

That your tests are working seems to suggest Vaex was also updated, and that we could lift the constraint

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

Successfully merging a pull request may close this issue.

2 participants