#18121: Patch Cython with PyTypeObject members
-------------------------------------+-------------------------------------
       Reporter:  jdemeyer           |        Owner:
           Type:  enhancement        |       Status:  needs_review
       Priority:  major              |    Milestone:  sage-6.6
      Component:  packages:          |   Resolution:
  standard                           |    Merged in:
       Keywords:  sd66               |    Reviewers:
        Authors:  Jeroen Demeyer     |  Work issues:
Report Upstream:  Fixed upstream,    |       Commit:
  but not in a stable release.       |  a10dcbaaaefb87d48cf337be367cd48f11ed6edf
         Branch:                     |     Stopgaps:
  u/jdemeyer/patch_cython_with_pytypeobject_members|
   Dependencies:                     |
-------------------------------------+-------------------------------------

Comment (by nbruin):

 Replying to [comment:19 vdelecroix]:
 > Why `void Py_VISIT(PyObject *o)` is not compatible with Cython? I do not
 quite understand what is this `__pyx_v_visit`. Is it because of Cython
 renaming arguments? If it is so, I do not find it very safe.

 Correct, and I agree. Depending on the user giving parameters a particular
 name ''and'' cython mangling the names in a particular way is more fragile
 than what the CPython macro does. We only use this stuff twice, so it's
 not too onerous and much less obscure to be explicit in the code. If we
 find ourselves writing more "traverse" functions, we should push for a
 more structural solution in cython (currently we're already hacking by
 modifying a pytype structure after !PyTypeReady has run on it).

 > Could you explain to me the reason of declaring
 > {{{
 >     int PyList_SetItem(object list, Py_ssize_t index, PyObject * item)
 except -1
 > }}}
 > and not use the `PyList_SetItem` from `cpython.lists` declared as
 `(object,Py_ssize_t,object)`? It would also be cool to add a comment if it
 is important to keep as such.

 See its use in `extract_*` functions. It's handling `PyObject*` there and
 a cast to `object` would generate an INCREF (which is really not what you
 want to do in code that's just deleting things). This ticket doesn't
 affect that code.
 > In `sage/structure/coerce_dict.pyx`, couldn't you get rid of
 > {{{
 > cdef extern from "Python.h":
 >     PyObject* PyWeakref_GetObject(object r)
 > }}}
 > It is in `cpython.weakref`.

 So it is now! I don't think it was before.

 > Do you know why `Py_None` is not available from Cython?

 It is as "None", but then it's an `object`, and we need to compare it as a
 `PyObject *`. Cython's support for `PyObject *` (i.e., borrowed
 references) is poor -- basically intentionally so at the moment because a
 good interface (other than just the C interface) hasn't been formulated
 yet.

 > Could we also remove or comment the `cpdef inline Py_ssize_t
 signed_id(x)`?

 No objections to that.

--
Ticket URL: <http://trac.sagemath.org/ticket/18121#comment:20>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica, 
and MATLAB

-- 
You received this message because you are subscribed to the Google Groups 
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.

Reply via email to