#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
.pxifiles fromsrc/sage/gslto.pxdfiles 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.pyto the new.pxdfiles.
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_doublethe dependency onpariandgslare now automatically added from the fact you import or declare some symbols in the corresponding.pxdfile?
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
pariinlibrariesbefore 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 ptestpasses. 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