Opened 6 years ago

Last modified 6 years ago

#20002 closed enhancement

Move interrupt.pyx to package cysignals — at Version 25

Reported by: malb Owned by:
Priority: major Milestone: sage-7.1
Component: packages: standard Keywords:
Cc: jdemeyer, vdelecroix, fbissey, vbraun Merged in:
Authors: Jeroen Demeyer Reviewers:
Report Upstream: Reported upstream. No feedback yet. Work issues:
Branch: u/jdemeyer/move_interrupt_pyx_to_package_cysignals (Commits, GitHub, GitLab) Commit: 434b272c137ab3ca014ce245df952793426e5afe
Dependencies: Stopgaps:

Status badges

Change History (25)

comment:1 Changed 6 years ago by vdelecroix

  • Cc vdelecroix added

I am interested, partly because of pplpy.

comment:2 Changed 6 years ago by jdemeyer

This is certainly something that I would like to work on, I think it fits very well with https://github.com/OpenDreamKit/OpenDreamKit/issues/52.

comment:3 Changed 6 years ago by malb

  • Description modified (diff)

comment:4 Changed 6 years ago by jdemeyer

Cool! I guess the next step is to figure out how to actually make this signals.pyx available to external packages (you need to install the .pxd, .pxi and .h files).

I think it would be very useful to have an example package using signals.pyx. Both as documentation, but also as an additional test.

comment:5 Changed 6 years ago by malb

I created a cysignals branch for fpylll which uses the new cysignals.

comment:6 Changed 6 years ago by malb

  • Description modified (diff)

comment:7 Changed 6 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Description modified (diff)
  • Summary changed from stand-alone interrupt.pyx to Move interrupt.pyx to package cysignals

comment:8 Changed 6 years ago by fbissey

  • Cc fbissey added

comment:9 Changed 6 years ago by jdemeyer

  • Dependencies set to #20021

comment:10 Changed 6 years ago by jdemeyer

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

comment:11 Changed 6 years ago by jdemeyer

  • Description modified (diff)

comment:12 Changed 6 years ago by jdemeyer

  • Branch set to u/jdemeyer/move_interrupt_pyx_to_package_cysignals

comment:13 Changed 6 years ago by git

  • Commit set to 434b272c137ab3ca014ce245df952793426e5afe

Branch pushed to git repo; I updated commit sha1. New commits:

aee6a5fCheck that sig_on()/sig_off() works in .spyx files
434b272cysignals depends on PARI

comment:14 Changed 6 years ago by jdemeyer

  • Status changed from new to needs_review

comment:15 Changed 6 years ago by jdemeyer

  • Cc vbraun added

@vbraun: please test on buildbots.

comment:16 follow-up: Changed 6 years ago by malb

Couldn't we use the include_path=sys.path trick to avoid patching Cython, just in case they don't accept the pull request?

comment:17 in reply to: ↑ 16 Changed 6 years ago by jdemeyer

Replying to malb:

Couldn't we use the include_path=sys.path trick to avoid patching Cython, just in case they don't accept the pull request?

The problem is that we have to support:

  1. the Sage library
  2. the cython(...) command
  3. the %cython notebook magic
  4. ./sage somefile.spyx
  5. external spkgs using cysignals

Instead of patching all these, I think it is much cleaner to apply just the Cython patch.

comment:18 Changed 6 years ago by malb

Makes sense to me.

comment:19 Changed 6 years ago by jdemeyer

  • Dependencies #20021 deleted

comment:20 Changed 6 years ago by malb

Building a fresh Sage installation and running make ptestlong works on 64-bit Debian/GNU Linux.

comment:21 follow-up: Changed 6 years ago by malb

  • What is our convention for dependencies in SPKG.txt, shall we mention Pari?
  • I'm no fan of "init_interrupts -> init_cysignals". Why not fix up cysignals if that's needed?

comment:22 Changed 6 years ago by malb

  • Status changed from needs_review to needs_info

comment:23 in reply to: ↑ 21 Changed 6 years ago by fbissey

Replying to malb:

  • What is our convention for dependencies in SPKG.txt, shall we mention Pari?

It is redundant with another file but since both python and cython are there, I guess it should be added.

  • I'm no fan of "init_interrupts -> init_cysignals". Why not fix up cysignals if that's needed?

It is fixed on master but not in 0.9. Got bitten by that when I tried to build the thing with sage-on-gentoo (does build now against cysignals master instead of patching 0.9. )

comment:24 Changed 6 years ago by jdemeyer

  • Status changed from needs_info to needs_work

comment:25 Changed 6 years ago by jdemeyer

  • Description modified (diff)
Note: See TracTickets for help on using tickets.