8000 scikit-learn runtime dependencies issues in setup.py · Issue #18977 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content
8000

scikit-learn runtime dependencies issues in setup.py #18977

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.

Sign up for GitHub

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

Jump to bottom
Closed
ogrisel opened this issue Dec 8, 2020 · 4 comments
Closed

scikit-learn runtime dependencies issues in setup.py #18977

ogrisel opened this issue Dec 8, 2020 · 4 comments
Labels
Blocker Bug Packaging
Milestone
0.24

Comments

@ogrisel
Copy link
Member
ogrisel commented Dec 8, 2020 •
edited
Loading

Packaging issue reported after 0.24.0rc1: #18956 (comment)

I think we have 2 problems:

  • we should depend on numpy < 1.20 for Python 3.6
  • we should not try to import numpy or build stuff with Cython and a c compiler in our setup.py just to compute the dependencies (if possible), for instance when running python setup.py check.
@ogrisel ogrisel added this to the 0.24 milestone Dec 8, 2020
@ogrisel ogrisel added the Blocker label Dec 8, 2020
@alfaro96
Copy link
Member
alfaro96 commented Dec 9, 2020 •
edited
Loading

Just to clarify myself:

Packaging issue reported after 0.24.0rc1: #18956 (comment)

I think we have 2 problems:

  • we should depend on numpy < 1.20 for Python 3.6

We specify in the setup.py file the minimum required version for NumPy and SciPy, but not the maximum one. I am not sure if we should specify this.

We should specify the maximum version to avoid compiling from source with Python 3.6.

  • we should not try to import numpy or build stuff with Cython and a c compiler in our setup.py just to compute the dependencies (if possible), for instance when running python setup.py check.

I think that adding the required commands here:

scikit-learn/setup.py

Lines 269 to 275 in 4773f3e

if len(sys.argv) == 1 or (
len(sys.argv) >= 2 and ('--help' in sys.argv[1:] or
sys.argv[1] in ('--help-commands',
'egg_info',
'dist_info',
'--version',
'clean'))):

should be enough. Is it not?

@thomasjpfan thomasjpfan mentioned this issue Dec 10, 2020
Closed
@ogrisel
Copy link
Member Author
ogrisel commented Dec 11, 2020

I think that adding the required commands here: [...] should be enough. Is it not?

Yes. I thought that setuptools (used by python setup.py develop of the third party project) would have used egg_info, so we should have been covered. This require more investigation. But adding check to the list of commands is probably a good idea.

@alfaro96 alfaro96 mentioned this issue Dec 14, 2020
Merged
@alfaro96
Copy link
Member

Should we close this blocker?

@ogrisel
Copy link
Member Author
ogrisel commented Dec 16, 2020

Yes.

@ogrisel ogrisel closed this as completed Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Blocker Bug Packaging
Projects
None yet
Milestone
0.24
Development

No branches or pull requests
2 participants
@ogrisel @alfaro96
0