#18778 closed enhancement (fixed)
Clean up GSL declarations
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.8 |
Component: | cython | Keywords: | |
Cc: | Merged in: | ||
Authors: | Jeroen Demeyer | Reviewers: | François Bissey |
Report Upstream: | N/A | Work issues: | |
Branch: | 6740862 (Commits) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
- Move all
.pxi
files fromsrc/sage/gsl
to.pxd
files in new directorysrc/sage/libs/gsl
. - Split off all declarations for common types in
src/sage/libs/gsl/types.pxd
- Move compile arguments from
src/module_list.py
to the new.pxd
files.
Change History (13)
comment:1 Changed 6 years ago by
- Summary changed from Move GSL pxi files to pxd files in libs/gsl to Clean up GSL declarations
comment:2 Changed 6 years ago by
- Branch set to u/jdemeyer/move_gsl_pxi_files_to_pxd_files_in_libs_gsl
comment:3 Changed 6 years ago by
- Commit set to 6740862036cff7d93756315929ead4461526dd6c
- Status changed from new to needs_review
comment:4 Changed 6 years ago by
- Description modified (diff)
comment:5 follow-ups: ↓ 6 ↓ 8 Changed 6 years ago by
Hum so in the case of sage/rings/complex_double
the dependency on pari
and gsl
are now automatically added from the fact you import or declare some symbols in the corresponding .pxd
file? Of course that's how the linking to pari must have been done in the first place since there was already no pari
in libraries
before your change. And thank you for demonstrating aliases
, I feel I will abuse this in the future.
Looks good but I should at least do a build of that one because it is a lot of changes even if they are seemingly trivial.
comment:6 in reply to: ↑ 5 ; follow-up: ↓ 7 Changed 6 years ago by
Replying to fbissey:
Hum so in the case of
sage/rings/complex_double
the dependency onpari
andgsl
are now automatically added from the fact you import or declare some symbols in the corresponding.pxd
file?
If you replace "import or declare" by "cimport
", yes, you are right. You just need to add the libraries to the .pxd
file.
Of course that's how the linking to pari must have been done in the first place since there was already no
pari
inlibraries
before your change.
That was done in #18450.
And thank you for demonstrating
aliases
, I feel I will abuse this in the future.
Abuse? :-)
Anyway, for #17630, you'll probably want to define an analogous BLAS_LIBRARIES
or something.
comment:7 in reply to: ↑ 6 Changed 6 years ago by
comment:8 in reply to: ↑ 5 ; follow-up: ↓ 9 Changed 6 years ago by
Replying to fbissey:
Looks good but I should at least do a build of that one because it is a lot of changes even if they are seemingly trivial.
On my Linux laptop, make ptest
passes. I'll try on an OS X box also.
comment:9 in reply to: ↑ 8 Changed 6 years ago by
Replying to jdemeyer:
Replying to fbissey:
Looks good but I should at least do a build of that one because it is a lot of changes even if they are seemingly trivial.
On my Linux laptop,
make ptest
passes. I'll try on an OS X box also.
I don't doubt that you have done so already. I just do what I think I should do as a reviewer, running my own independent tests in this case.
comment:10 Changed 6 years ago by
Everything fine on OS X also.
comment:11 Changed 6 years ago by
- Reviewers set to François Bissey
- Status changed from needs_review to positive_review
Ready to go ahead. Have a good day.
comment:12 Changed 6 years ago by
- Branch changed from u/jdemeyer/move_gsl_pxi_files_to_pxd_files_in_libs_gsl to 6740862036cff7d93756315929ead4461526dd6c
- Resolution set to fixed
- Status changed from positive_review to closed
comment:13 Changed 6 years ago by
- Commit 6740862036cff7d93756315929ead4461526dd6c deleted
Thanks for the review!
New commits:
Clean up GSL includes