Opened 2 years ago

Closed 18 months ago

Last modified 4 months ago

#23600 closed defect (duplicate)

Fix the remaining cpdef declaration issues concerning vtables

Reported by: roed Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: cython Keywords:
Cc: jdemeyer Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

In #23227 we added cpdef declarations to address most of https://github.com/cython/cython/issues/1732. A few were not completely straightforward and will be dealt with in follow-up tickets:

  1. sage/algebras/quatalg/quaternion_algebra_element.pxd: #24607
  1. sage/modular/pollack_stevens: #23603
  1. sage/modules: #24607
  1. sage/numerical/backends: #20324
  1. sage/rings/finite_rings: #24116 and #24607
  1. sage/rings/number_field: #23603
  1. sage/rings/padics: #27417, #24609
  1. sage/rings/polynomial/laurent_polynomial.pxd: #23606
  1. sage/rings/power_series_*: #24607 and #24608

Change History (23)

comment:1 Changed 2 years ago by roed

Jeroen, can you add the code here that you removed from the other branch?

comment:2 Changed 2 years ago by jdemeyer

I would rather prefer to fix this on a case-by-case basis (for example, one ticket for p-adics, one ticket for quaternion algebras, ...)

comment:3 Changed 2 years ago by jdemeyer

  • Branch set to u/jdemeyer/fix_the_remaining_cpdef_declaration_issues

comment:4 Changed 2 years ago by jdemeyer

  • Commit set to 7fdcbe7a64ce7bc55d715e76f2d2cf7d4d342e74

New commits:

7fdcbe7Add cpdef _add_(self, other) and cpdef _mul_(self, other) all over

comment:5 Changed 2 years ago by jdemeyer

To be clear: I am not saying that I'm against all the changes in this remaining branch, only that it's not immediately clear that it's the right thing to do.

comment:6 Changed 2 years ago by jdemeyer

Comments in more detail:

  1. sage/algebras/quatalg: instead, add abstract methods _add_ and _mul_ to the base class.
  1. sage/modular/pollack_stevens: unrelated change to cpdef normalize
  1. src/sage/modules: instead, add abstract methods _add_ and _mul_ to the base class.
  1. sage/numerical/backends: add_variable is already declared in the base class GenericBackend
  1. sage/rings/finite_rings: instead, add abstract methods _add_ and _mul_ to the base class.
  1. src/sage/rings/number_field: already in the base class NumberFieldElement (but not declared there)
  1. sage/rings/padics: lots of changes, not looked in detail yet.
  1. src/sage/rings/polynomial and power series: instead, add abstract methods to the base class.
Last edited 2 years ago by jdemeyer (previous) (diff)

comment:7 Changed 2 years ago by jdemeyer

I can try to fix some of these, but we should coordinate to avoid double work. Items 2, 4 and 6 on the list above seem easiest, so I will have a quick look now.

EDIT: 2 and 4 are different cpdef declarations issues, they are because of a changed signature. I sent an e-mail to cython-users asking whether the warning is still correct in this case.

Last edited 2 years ago by jdemeyer (previous) (diff)

comment:8 Changed 2 years ago by roed

Okay. I'm traveling at the moment, so I'm not going to work on this immediately. I'll make a comment here before doing anything.

comment:9 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:10 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:11 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:12 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:13 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:14 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:15 Changed 2 years ago by jdemeyer

  • Description modified (diff)

comment:16 Changed 21 months ago by jdemeyer

  • Description modified (diff)

comment:17 Changed 18 months ago by jdemeyer

  • Description modified (diff)

comment:18 Changed 18 months ago by jdemeyer

  • Description modified (diff)

comment:19 Changed 18 months ago by jdemeyer

  • Description modified (diff)

comment:20 Changed 18 months ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-8.1 to sage-duplicate/invalid/wontfix
  • Resolution set to duplicate
  • Status changed from new to closed

Every issue has a ticket now, so let's close this.

comment:21 Changed 18 months ago by jdemeyer

  • Branch u/jdemeyer/fix_the_remaining_cpdef_declaration_issues deleted
  • Commit 7fdcbe7a64ce7bc55d715e76f2d2cf7d4d342e74 deleted

comment:22 Changed 5 months ago by jdemeyer

  • Summary changed from Fix the remaining cpdef declaration issues to Fix the remaining cpdef declaration issues concerning vtables

comment:23 Changed 4 months ago by jdemeyer

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