Opened 9 years ago

Closed 9 years ago

#10493 closed enhancement (fixed)

Upgrade Cython to 0.14.1

Reported by: jason Owned by: tbd
Priority: blocker Milestone: sage-4.7
Component: packages: standard Keywords: cython spkg
Cc: robertwb, fbissey Merged in: sage-4.7.alpha1
Authors: Robert Bradshaw Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Version 0.14.1 was recently released, and has some bugfixes and nice features.

Use the following spkg and patch:

  1. http://boxen.math.washington.edu/home/jdemeyer/spkg/cython-0.14.1.p3.spkg
  2. 10493-cython-0.14.1.patch

Attachments (2)

10493-cython-0.14.1.patch (2.0 KB) - added by robertwb 9 years ago.
cython-SPKG.txt.diff (724 bytes) - added by jdemeyer 9 years ago.

Download all attachments as: .zip

Change History (32)

comment:1 follow-up: Changed 9 years ago by robertwb

We're in pretty good shape. There's a couple of minor patches to the library that need to go in, but other than that upgrading to 0.14 should be pretty straightforward. See, e.g. https://sage.math.washington.edu:8091/hudson/view/ExtLibs/job/sage-tests/

comment:2 Changed 9 years ago by jdemeyer

  • Keywords cython spkg added

Is the __getattr__ patch (in the 0.13 Cython spkg) still needed?

comment:3 Changed 9 years ago by robertwb

No, that is now handled via a flag. There are several bugfixes in 0.14.1 that I'd like to get in, which should be out shortly (before 4.6.2 is at release candidate for sure, and I knew we missed the window for 4.6.1) which is why I haven't done anything on this ticket yet.

comment:4 Changed 9 years ago by vbraun

Updated package with just the __getattr__ patches removed as they are now in the upstream source:

http://www.stp.dias.ie/~vbraun/Sage/spkg/cython-0.14.spkg

comment:5 Changed 9 years ago by robertwb

Thanks, but that is not sufficient, as we need to pass the appropriate flags.

The most recent spkg can be found at https://sage.math.washington.edu:8091/hudson/job/sage-build/lastSuccessfulBuild/artifact/cython-devel.spkg (though the Sage library requires a couple of patches as well).

comment:6 Changed 9 years ago by jason

  • Description modified (diff)
  • Summary changed from Upgrade Cython to 0.14 to Upgrade Cython to 0.14.1

Updating the description to upgrade to 0.14.1, which is now the most recent release. Has anyone worked on an 0.14.1 spkg?

Changed 9 years ago by robertwb

comment:7 Changed 9 years ago by robertwb

  • Status changed from new to needs_review

comment:8 Changed 9 years ago by fbissey

  • Cc fbissey added

comment:9 in reply to: ↑ 1 Changed 9 years ago by jason

Replying to robertwb:

There's a couple of minor patches to the library that need to go in

Is this still the case with the 0.14.1 spkg?

comment:10 Changed 9 years ago by fbissey

I think that's what the patch is all about. I applied it to 4.6.2.alpha4 and it happily built and sage -testall did not show any problems for me. But may be the following warnings should be addressed

warning: sage/combinat/combinat_cython.pyx:14:0: 'stdlib' is deprecated, use 'libc.stdlib'
warning: sage/ext/interpreters/wrapper_rdf.pxd:3:0: 'python_object' is deprecated, use 'cpython'
warning: sage/rings/polynomial/polynomial_real_mpfr_dense.pyx:7:0: 'python_int' is deprecated, use 'cpython'
warning: sage/rings/polynomial/polynomial_real_mpfr_dense.pyx:8:0: 'python_float' is deprecated, use 'cpython'
warning: sage/ext/interpreters/wrapper_cdf.pxd:3:0: 'python_object' is deprecated, use 'cpython'
warning: sage/ext/interpreters/wrapper_rdf.pxd:3:0: 'python_object' is deprecated, use 'cpython'
warning: sage/ext/interpreters/wrapper_py.pxd:3:0: 'python_object' is deprecated, use 'cpython'
warning: sage/ext/interpreters/wrapper_rr.pxd:3:0: 'python_object' is deprecated, use 'cpython'
warning: sage/ext/interpreters/wrapper_el.pxd:3:0: 'python_object' is deprecated, use 'cpython'

And possibly

warning: sage/modular/modsym/p1list.pyx:703:17: cdef variable 'i' declared after it is used

Finally I am wondering about these

warning: sage/matrix/matrix0.pyx:53:22: Function signature does not match previous declaration
warning: sage/matrix/matrix_window.pyx:9:17: Function signature does not match previous declaration
warning: sage/matrix/matrix_sparse.pyx:18:33: Function signature does not match previous declaration

But all these were present before upgrading to cython-0.14.1. We could use the opportunity to fix some of them before they bite us later, especially the deprecated ones.

comment:11 Changed 9 years ago by fbissey

Forgot these two:

warning: sage/structure/parent.pyx:124:4: dict already a builtin Cython type
warning: sage/symbolic/pynac.pyx:589:5: Function 'py_get_parent_char' previously declared as 'extern'

comment:12 Changed 9 years ago by robertwb

Yes, the (attached) patch is still needed. There's some significant cleanup that I want to do, including fixing these warnings, but I think that should be a separate ticket.

comment:13 Changed 9 years ago by fbissey

  • Description modified (diff)
  • Status changed from needs_review to positive_review

Well if you open a separate ticket for these warnings I am quite happy to give this a positive review. I have built it on linux-86, linux-amd64 and OS X 10.5. It all looks good to me.

comment:14 Changed 9 years ago by robertwb

Ticket for warnings at #10764.

comment:15 Changed 9 years ago by jdemeyer

  • Authors set to Robert Bradshaw
  • Milestone changed from sage-4.6.2 to sage-4.7
  • Reviewers set to François Bissey

comment:16 Changed 9 years ago by jdemeyer

  • Status changed from positive_review to needs_work

After building sage-4.6.2.rc0 and applying the spkg and patch from this ticket:

$ ./sage -ba-force
[...]
gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -O3 -Wall -Wstrict-prototypes -fPIC -I/usr/local/src/sage-4.6.2.rc0/local//include -I/usr/local/src/sage-4.6.2.rc0/local//include/csage -I/usr/local/src/sage-4.6.2.rc0/devel//sage/sage/ext -I/usr/local/src/sage-4.6.2.rc0/local/include/python2.6 -c sage/combinat/combinat_cython.c -o build/temp.linux-x86_64-2.6/sage/combinat/combinat_cython.o -w
sage/combinat/combinat_cython.c: In function '__pyx_pf_4sage_8combinat_15combinat_cython__stirling_number2':
sage/combinat/combinat_cython.c:2431:13: error: incompatible types when assigning to type 'mpz_t' from type 'struct __mpz_struct *'
error: command 'gcc' failed with exit status 1
sage: There was an error installing modified sage library code.

This is using

$ gcc --version
gcc-4.4.3 (Gentoo 4.4.3-r2 p1.2) 4.4.3
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

comment:17 Changed 9 years ago by jason

Why is the deprecation warning in polynomial_compiled.pyx commented out in this patch? If it is not being used, shouldn't it just be removed?

comment:18 Changed 9 years ago by robertwb

I'm not sure why the gcc error--I'll take a look at that. (I'm guessing that's some new code?)

As for commenting out the deprecation warning, the issue is that decorators aren't implemented for Cython special functions. They used to be silently ignored, but now an error is raised. When they're supported, we should put this back in.

comment:19 follow-up: Changed 9 years ago by robertwb

Somehow one of the fixes for http://trac.cython.org/cython_trac/ticket/654 was merged into the release at the last minute, which is incompatible with how we declare many of the *_t types. I'll post a new spkg disabling this "fix."

comment:20 Changed 9 years ago by robertwb

  • Description modified (diff)
  • Status changed from needs_work to needs_review

I've created a new spkg. All tests pass.

comment:21 in reply to: ↑ 19 Changed 9 years ago by fbissey

Replying to robertwb:

Somehow one of the fixes for http://trac.cython.org/cython_trac/ticket/654 was merged into the release at the last minute, which is incompatible with how we declare many of the *_t types. I'll post a new spkg disabling this "fix."

How come you and I never got affected by it?

comment:22 Changed 9 years ago by robertwb

I'm not sure, but maybe the last spkg was the rc, and the fix for this issue was not supposed to go in this release, but we messed up and it did.

comment:23 Changed 9 years ago by jdemeyer

  • Status changed from needs_review to positive_review

It works for me now, so I'm putting back the positive review.

comment:24 Changed 9 years ago by robertwb

Thanks.

comment:25 Changed 9 years ago by jdemeyer

  • Merged in set to sage-4.7.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:26 Changed 9 years ago by jdemeyer

  • Priority changed from major to blocker
  • Resolution fixed deleted
  • Status changed from closed to new

Please fix: the SPKG.txt must be updated, there is no mention of this ticket.

comment:27 Changed 9 years ago by robertwb

  • Description modified (diff)
  • Status changed from new to needs_review

Changed 9 years ago by jdemeyer

comment:28 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:29 Changed 9 years ago by robertwb

  • Status changed from needs_review to positive_review

I'm happy with your spkg.txt.

comment:30 Changed 9 years ago by jdemeyer

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.