Opened 7 years ago

Closed 7 years ago

#17726 closed enhancement (fixed)

Replace PY_TYPE() by type()

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.5
Component: cython Keywords:
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: Marc Mezzarobba
Report Upstream: N/A Work issues:
Branch: 05e0633 (Commits, GitHub, GitLab) Commit: 05e0633a1e366dacf7337b2f4895daf137b38bad
Dependencies: Stopgaps:

Status badges

Description


Change History (7)

comment:1 Changed 7 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/17726
  • Created changed from 02/04/15 09:29:54 to 02/04/15 09:29:54
  • Modified changed from 02/04/15 09:29:54 to 02/04/15 09:29:54

comment:2 Changed 7 years ago by jdemeyer

  • Commit set to 05e0633a1e366dacf7337b2f4895daf137b38bad
  • Status changed from new to needs_review

New commits:

​05e0633Replace PY_TYPE()

comment:3 follow-up: Changed 7 years ago by vdelecroix

These are not all... 

$ grep -lR "PY_TYPE("
matrix/matrix_window.pyx
symbolic/function.pxd
categories/map.pyx
ext/stdsage.pxi
structure/parent.pyx
structure/coerce.pyx
rings/number_field/number_field_element.pyx
rings/finite_rings/integer_mod.pyx
rings/polynomial/polynomial_modn_dense_ntl.pyx
rings/integer.pyx

Some of them are already in #17727. Could we set the latter as a dependency and completely get rid of PY_TYPE?

Vincent

comment:4 Changed 7 years ago by vdelecroix

  • Status changed from needs_review to needs_info

comment:5 in reply to: ↑ 3 ; follow-up: Changed 7 years ago by mmezzarobba

  • Reviewers set to Marc Mezzarobba
  • Status changed from needs_info to positive_review

Replying to vdelecroix:

Some of them are already in #17727. Could we set the latter as a dependency and completely get rid of PY_TYPE?

With #17668 + #17725 + #17726 + #17727, the only remaining occurrences of PY_TYPE are in stdsage.*. And the earlier in the release cycle patches of this kind get merged, the better. So if you don't mind let's leave the issue of removing PY_TYPE itself for later, as the present ticket makes sense without it.

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

Replying to mmezzarobba:

Replying to vdelecroix:

Some of them are already in #17727. Could we set the latter as a dependency and completely get rid of PY_TYPE?

With #17668 + #17725 + #17726 + #17727, the only remaining occurrences of PY_TYPE are in stdsage.*.

Indeed, I've made these different ticket independent for easier reviewing.

If all four of the above tickets are merged, I plan to make a ticket to remove PY_TYPE_CHECK and another one to remove the stdsage.pxi includes.

I would keep the functions in stdsage.pxi for now, just to support existing code on Trac which uses them (consider them "deprecated").

comment:7 Changed 7 years ago by vbraun

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