Opened 4 years ago

Closed 4 years ago

#18145 closed enhancement (fixed)

Cythonize optional extensions

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.6
Component: build Keywords:
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: e56aeb2 (Commits) Commit: e56aeb2c9a190813096a0f7db1683d3d5bc774d1
Dependencies: #17860, #18095 Stopgaps:

Description

For Cython code dependending on optional packages, we can still run cython on that .pyx file. The advantage is that breakage due to Cython changes will be detected earlier.

One more advantage of this branch is that optional extensions are listed in between the normal extensions, which is much cleaner.

Change History (10)

comment:1 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/18145

comment:2 Changed 4 years ago by jdemeyer

  • Commit set to 4105d78f8f5a03fbba8cf12b372e048dde3aa022
  • Status changed from new to needs_review

New commits:

4105d78Cythonize optional extensions

comment:3 Changed 4 years ago by mmezzarobba

  • Status changed from needs_review to needs_work
  • Work issues set to conflicts with #18095

comment:4 Changed 4 years ago by jdemeyer

  • Dependencies set to #17860, #18095

comment:5 Changed 4 years ago by git

  • Commit changed from 4105d78f8f5a03fbba8cf12b372e048dde3aa022 to e56aeb2c9a190813096a0f7db1683d3d5bc774d1

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c98e78dMerge commit '7d1b5f8ca56180ca2d7044453707c619ef17b51a' into HEAD
c683c19Add PARI documentation
4b07161Better format links
9f710a2Merge commit '4b071619a00e667d3452a73229de43f231dfe662' into HEAD
41755feRe-organize building of Sage library and auto-generated files
3510d45Minor review fixes
e56aeb2Cythonize optional extensions

comment:6 Changed 4 years ago by jdemeyer

  • Status changed from needs_work to needs_review
  • Work issues conflicts with #18095 deleted

comment:7 follow-up: Changed 4 years ago by fbissey

OK so if (in the general case) the spkg is installed OptionalExtension returns Extension but what does CythonizeExtension which is the alternative result does?

comment:8 in reply to: ↑ 7 Changed 4 years ago by jdemeyer

Replying to fbissey:

OK so if (in the general case) the spkg is installed OptionalExtension returns Extension but what does CythonizeExtension which is the alternative result does?

I cannot really parse your question, but I'll try to answer it anyway: CythonizeExtension is exactly like Extension except for the skip_build attribute. There is some code added to src/setup.py which checks this attribute. If skip_build, then the build (i.e. the foo.c -> foo.so part) of the extension is skipped.

comment:9 Changed 4 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

Thanks. You are doing an extremely good job of answering questions you cannot parse :) It otherwise looks good to me and should go in. I must say I thought something needed to be done with the optional stuff and now it is!

comment:10 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/18145 to e56aeb2c9a190813096a0f7db1683d3d5bc774d1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.