Opened 4 years ago

Closed 4 years ago

#23195 closed enhancement (fixed)

Stop using cysignals .pxi files in interpreters

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.0
Component: cython Keywords:
Cc: embray Merged in:
Authors: Jeroen Demeyer Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: be167ac (Commits, GitHub, GitLab) Commit: be167ac79e68f3beb9a7c1bd38216fb5bad6349e
Dependencies: #23196 Stopgaps:

Status badges

Description

Follow-up to #22806 and #22896: we should stop using the deprecated files cysignals/memory.pxi and cysignals/signals.pxi.

This deals with the auto-generated interpreters files. Besides changing the include to a cimport, we also clean up and optimize the memory allocation code a bit.

Change History (9)

comment:1 Changed 4 years ago by jdemeyer

  • Dependencies set to #23196

comment:2 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/stop_using_cysignals__pxi_files_in_interpreters

comment:3 Changed 4 years ago by jdemeyer

  • Commit set to be167ac79e68f3beb9a7c1bd38216fb5bad6349e
  • Status changed from new to needs_review

New commits:

78abaaeFix doctest parsing with triple quotes inside parentheses
be167acStop using cysignals .pxi files in interpreters

comment:4 follow-up: Changed 4 years ago by embray

I don't really know enough about the differences between the old and new APIs to really have an opinion about this, but it seems obvious and straightforward enough.

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

Replying to embray:

I don't really know enough about the differences between the old and new APIs

The main goal here is getting rid of the of Cython include statements, which are semi-deprecated(*) by Cython upstream.

The change from sig_malloc to check_allocarray is just a minor clean-up which removes the need for the NULL check.

(*) They would like to deprecate include statements. However, there are some corner cases where include statements make sense, so they don't formally deprecate them. Instead, they just keep them working in the current state, rejecting any new features.

comment:6 Changed 4 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:7 Changed 4 years ago by jdemeyer

  • Dependencies #23196 deleted

This doesn't strictly depend on #23196.

comment:8 Changed 4 years ago by jdemeyer

  • Dependencies set to #23196

Actually, it does depend since this branch contains the commit from #23196.

comment:9 Changed 4 years ago by vbraun

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