Opened 5 years ago
Closed 5 years ago
#18142 closed enhancement (fixed)
Numpy: fix dependency checking of headers for Sage library code
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage6.6 
Component:  packages: standard  Keywords:  
Cc:  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  François Bissey 
Report Upstream:  N/A  Work issues:  
Branch:  d9bbc1d (Commits)  Commit:  d9bbc1d673590fcd18db88120ac95c68a25f281a 
Dependencies:  Stopgaps: 
Description (last modified by )
 In
src/setup.py
, usenumpy.get_include()
to determine the location of thenumpy
includes.  When installing
numpy
, touch the includes to fix dependency checking.  Remove all the manual numpy header and dependency stuff from
src/module_list.py
.
The removal of the manual numpy
dependencies only removed the numpy
dependency of src/sage/finance/fractal.pyx
(which doesn't use numpy
, so the change is correct).
Change History (18)
comment:1 Changed 5 years ago by
 Description modified (diff)
comment:2 Changed 5 years ago by
 Description modified (diff)
comment:3 followup: ↓ 5 Changed 5 years ago by
comment:4 Changed 5 years ago by
numpy.get_include()
is what we are after.
comment:5 in reply to: ↑ 3 Changed 5 years ago by
Replying to fbissey:
Of course with sage you can afford the shortcut to
local/include
I cannot when I can numpy installed for various versions of python. I'd like to see if numpy provides a mechanism first.
François, I don't quite understand what you're trying to say. Are you saying that it's a bad idea to symlink numpy's headers in $SAGE_LOCAL/include/numpy
?
comment:6 followup: ↓ 7 Changed 5 years ago by
Well for sage alone you don't really care. But from my sageondistro point of view I can have numpy for python 2.7.9 and python 3.4.3 installed. And I will have to choose which one to link under /usr/include
. But I think the annoyance we have in setup.py (and currently module_list.py), is because we never bothered to look if numpy itself could just tell us where its headers are. And as a matter of fact it can.
comment:7 in reply to: ↑ 6 Changed 5 years ago by
Replying to fbissey:
Well for sage alone you don't really care. But from my sageondistro point of view I can have numpy for python 2.7.9 and python 3.4.3 installed.
Does the Python version even matter for the C includes of numpy
?
comment:8 followup: ↓ 10 Changed 5 years ago by
If you prefer numpy.get_include()
, that could be arranged. But I still find it strange that every single package installs its includes in $SAGE_LOCAL/includes
except for numpy.
comment:9 Changed 5 years ago by
 Description modified (diff)
 Summary changed from Numpy: install includes in $SAGE_LOCAL/include to Numpy: fix dependency checking of headers
comment:10 in reply to: ↑ 8 Changed 5 years ago by
Replying to jdemeyer:
If you prefer
numpy.get_include()
, that could be arranged. But I still find it strange that every single package installs its includes in$SAGE_LOCAL/includes
except for numpy.
The only python packages that I know have headers and install under include
are
 python itself under python$version
 pynac (python 2.7 only)
 polybori (python 2.7 only)
Apart from numpy and cython I cannot think of any other python packages that install headers and those two definitely don't do it under $prefix/include
. Because some of the headers in these can depend on the version of python I don't think it will happen anytime soon.
comment:11 Changed 5 years ago by
 Branch set to u/jdemeyer/ticket/18142
comment:12 Changed 5 years ago by
 Commit set to d9bbc1d673590fcd18db88120ac95c68a25f281a
 Status changed from new to needs_review
New commits:
d9bbc1d  Simplify building Cython code which uses numpy

comment:13 followup: ↓ 14 Changed 5 years ago by
After inspection of my system I got one header that is shared between version of python under /usr/include
: pygobject/pygobject.h
. There is only one switch inside that depends on the version of python used if I read the thing correctly. So it can be done.
Anyway the patch looks very good to me and it get the right head counts of numpy_depends
(26) in module_list.py
.
The only thing you forgot is that bit of code in sage/misc/cython.py
include_dirs = [os.path.join(SAGE_LOCAL,'include','csage'), os.path.join(SAGE_LOCAL,'include'), \ os.path.join(SAGE_LOCAL,'include','python'+platform.python_version().rsplit('.', 1)[0]), \ os.path.join(SAGE_LOCAL,'lib','python','sitepackages','numpy','core','include'), \ os.path.join(SAGE_SRC,'sage','ext'), \ os.path.join(SAGE_SRC), \ os.path.join(SAGE_SRC,'sage','gsl')]
comment:14 in reply to: ↑ 13 Changed 5 years ago by
Replying to fbissey:
The only thing you forgot is that bit of code in
sage/misc/cython.py
Please no! That's outside the scope of this ticket. Fixing the big mess which is sage/misc/cython.py
is for later.
comment:15 Changed 5 years ago by
 Summary changed from Numpy: fix dependency checking of headers to Numpy: fix dependency checking of headers for Sage library code
comment:16 followup: ↓ 17 Changed 5 years ago by
 Reviewers set to François Bissey
 Status changed from needs_review to positive_review
Fair enough cython.py deserve its own ticket.
comment:17 in reply to: ↑ 16 Changed 5 years ago by
Replying to fbissey:
Fair enough cython.py deserve its own ticket.
But later. There are way too many Cythonrelated tickets that it's becoming hard to do anything which doesn't conflict with another ticket.
comment:18 Changed 5 years ago by
 Branch changed from u/jdemeyer/ticket/18142 to d9bbc1d673590fcd18db88120ac95c68a25f281a
 Resolution set to fixed
 Status changed from positive_review to closed
It always felt strange that there isn't a way to get to the numpy include location from numpy itself. Is it just because we are ignorant? If not it seems to be a case of something missing to me.
Of course with sage you can afford the shortcut to
local/include
I cannot when I can numpy installed for various versions of python. I'd like to see if numpy provides a mechanism first.