Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#13029 closed enhancement (fixed)

Upgrade Cython to 0.17

Reported by: robertwb Owned by: tbd
Priority: major Milestone: sage-5.2
Component: packages: standard Keywords:
Cc: jdemeyer, roed, ohanar, ppurka, kini Merged in: sage-5.2.beta1
Authors: Robert Bradshaw Reviewers: R. Andrew Ohana
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

It's not released yet, but the following spkg should be close and the patch needs reviewing.

Apply:

and install http://sage.math.washington.edu/home/robertwb/cython/cython-0.17pre.spkg

Attachments (3)

13029_cython-0.17-bug.patch (778 bytes) - added by robertwb 7 years ago.
13029-ignore-gen.h.patch (476 bytes) - added by robertwb 7 years ago.
13029_cython-0.17.patch (43.1 KB) - added by jdemeyer 7 years ago.

Download all attachments as: .zip

Change History (30)

Changed 7 years ago by robertwb

comment:2 Changed 7 years ago by robertwb

  • Status changed from new to needs_review

comment:3 Changed 7 years ago by ohanar

  • Status changed from needs_review to needs_work

Please describe why the hack in sage.rings.integer is needed.

comment:4 Changed 7 years ago by kini

  • Cc kini added
  • Description modified (diff)

comment:5 Changed 7 years ago by robertwb

  • Description modified (diff)

comment:6 Changed 7 years ago by robertwb

  • Status changed from needs_work to needs_review

comment:7 follow-up: Changed 7 years ago by ohanar

  • Status changed from needs_review to needs_work

on sage.math everything compiles fine, but upon startup

ImportError
...
/home/ohanar/sage-dev/sage-5.1.beta2/local/lib/python2.7/site-packages/sage/modular/arithgroup/all.py in <module>()
     15                             degeneracy_coset_representatives_gamma1)
     16 
---> 17 from farey_symbol import Farey as FareySymbol
     18 
     19 

ImportError: /home/ohanar/sage-dev/sage-5.1.beta2/local/lib/python2.7/site-packages/sage/modular/arithgroup/farey_symbol.so: undefined symbol: convert_to_long
Error importing ipy_profile_sage - perhaps you should run %upgrade?
WARNING: Loading of ipy_profile_sage failed.

This was from a clean sage source tarball built with the new cython.

Last edited 7 years ago by ohanar (previous) (diff)

comment:8 Changed 7 years ago by ohanar

  • Reviewers set to R. Andrew Ohana

comment:9 in reply to: ↑ 7 Changed 7 years ago by ohanar

  • Status changed from needs_work to needs_review

Replying to ohanar:

on sage.math everything compiles fine, but upon startup ...

wow, I can't read :p, testing now

comment:10 Changed 7 years ago by ohanar

  • Status changed from needs_review to needs_work

Please update the SPKG.txt in the spkg, otherwise everything looks good. I would hold out on making a new spkg until you can fix #13031, since that is one of the primary reasons for upgrading.

comment:11 Changed 7 years ago by ppurka

Applied the patch + spkg to Sage 5.1beta2, and it all works good. make ptestlong passed all tests.

comment:12 Changed 7 years ago by ohanar

  • Work issues set to Update SPKG.txt

FYI, it still works in beta5. Once you update the SPKG.txt, you may mark this with a positive review.

Last edited 7 years ago by ohanar (previous) (diff)

comment:13 Changed 7 years ago by robertwb

  • Status changed from needs_work to positive_review

Updated SPKG.txt and re-uploaded spkg.

comment:14 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.1 to sage-5.2
  • Work issues Update SPKG.txt deleted

comment:15 Changed 7 years ago by jdemeyer

  • Status changed from positive_review to needs_work

Why the change from

cdef void _pari_trap "_pari_trap" (long errno, long retries) except *: 

to

cdef public void _pari_trap "_pari_trap" (long errno, long retries) except *: 

This causes a file sage/libs/pari/gen.h to be generated. Either this file should not be distributed, or it should be added to .hgignore.

comment:16 Changed 7 years ago by robertwb

  • Status changed from needs_work to needs_review

It's public to prevent it from being declared as static which conflicts with the declaration in pari_err.h. I've added an entry to .hgignore.

comment:17 Changed 7 years ago by robertwb

  • Description modified (diff)

Changed 7 years ago by robertwb

comment:18 Changed 7 years ago by ohanar

  • Status changed from needs_review to positive_review

Makes sense.

Changed 7 years ago by jdemeyer

comment:19 Changed 7 years ago by jdemeyer

  • Description modified (diff)

Patch rebased to sage-5.2.beta0.

comment:20 Changed 7 years ago by fbissey

Just a quick question Robert, where can I find the sources matching the spkg? Is a real cython 0.17 going to be released soon?

comment:21 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.2.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:22 Changed 7 years ago by robertwb

For the record, the spkg is based on the commit at https://github.com/cython/cython/commit/a7d6ec066e480eb4cf2cdebd5392e0c674b83f96

We don't have a timeline for the 0.17 release (it's being worked on, intermittently), so we shouldn't wait on that.

comment:23 Changed 7 years ago by kini

Then maybe this SPKG should be versioned cython-0.16-a7d6ec0?

comment:24 Changed 7 years ago by robertwb

Fine by me, I'm just happy to see this finally get in :). When Cython 0.17 is finally released, it should be a trivial upgrade.

comment:25 Changed 7 years ago by fbissey

It is just a problem sage-on-gentoo side because I don't have an upstream package. I will have to distribute an ebuild fetching the spkg until 0.17 is released. If there was a snapshot on the cython website I would prefer to use that.

Last edited 7 years ago by fbissey (previous) (diff)

comment:27 in reply to: ↑ 26 Changed 7 years ago by fbissey

Replying to robertwb:

http://cython.org/release/Cython-0.17pre.tar.gz

I am grateful that you put that up Robert. Thanks a lot.

Note: See TracTickets for help on using tickets.