Opened 7 years ago
Closed 7 years ago
#17727 closed enhancement (fixed)
Remove redundant parent_c() functions
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage6.5 
Component:  coercion  Keywords:  
Cc:  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Marc Mezzarobba 
Report Upstream:  N/A  Work issues:  
Branch:  3a2e193 (Commits, GitHub, GitLab)  Commit:  3a2e193c06f8b26ff18c9555460209a62e54f3db 
Dependencies:  Stopgaps: 
Description (last modified by )
In Sage, we have parent_c
defined in 4 places, remove all except for sage/structure/element.pxd
We copy a shortcut from sage/symbolic/function.pxd
for numbers not inheriting from Element
(in particular Python int
and float
): we assume that these do not have a parent.
Change History (12)
comment:1 Changed 7 years ago by
 Description modified (diff)
comment:2 Changed 7 years ago by
 Description modified (diff)
comment:3 Changed 7 years ago by
 Branch set to u/jdemeyer/ticket/17727
 Created changed from 02/04/15 09:36:33 to 02/04/15 09:36:33
 Modified changed from 02/04/15 11:12:16 to 02/04/15 11:12:16
comment:4 Changed 7 years ago by
 Commit set to 3547ea8990d2c2f76001df02dac213956c515e07
 Status changed from new to needs_review
comment:5 in reply to: ↑ description Changed 7 years ago by
Replying to jdemeyer:
We copy a shortcut from
sage/symbolic/function.pxd
for numbers not inheriting fromElement
(in particular Pythonint
andfloat
): we assume that these do not have a parent.
IMHO it would be nice to add a note to that effect in the docstring of s.s.element.parent
. (Not that I really think anyone assigns parents to objects derived from builtin number types... but the concept is important enough that subtle points in its definition should be documented.)
comment:6 followup: ↓ 7 Changed 7 years ago by
According to Python documentation
int PyNumber_Check(PyObject *o) Returns 1 if the object o provides numeric protocols, and false otherwise. This function always succeeds.
So even sage integers, floating points numbers and many others will answer true to that (I checked). And from the specification I do not see how such test can be fast. I do not have anything against using PyNumber_Check
but the associated comment is definitely not appropriate.
Related question: why should we check for a method .parent()
? Would the following be invalid?
cdef parent_c(x): if isintance(x, Element): return (<Element> x)._parent else: return type(x)
Vincent
comment:7 in reply to: ↑ 6 ; followup: ↓ 8 Changed 7 years ago by
Replying to vdelecroix:
According to Python documentation
int PyNumber_Check(PyObject *o) Returns 1 if the object o provides numeric protocols, and false otherwise. This function always succeeds.So even sage integers, floating points numbers and many others will answer true to that (I checked). And from the specification I do not see how such test can be fast.
I don't understand your comment. As far as I understand PyNumber_Check
should amount to check if the corresponding PyTypeObject
's tp_as_number
is nonNULL
, and that should indeed be pretty fast.
comment:8 in reply to: ↑ 7 ; followup: ↓ 9 Changed 7 years ago by
Replying to mmezzarobba:
Replying to vdelecroix:
According to Python documentation
int PyNumber_Check(PyObject *o) Returns 1 if the object o provides numeric protocols, and false otherwise. This function always succeeds.So even sage integers, floating points numbers and many others will answer true to that (I checked). And from the specification I do not see how such test can be fast.
I don't understand your comment. As far as I understand
PyNumber_Check
should amount to check if the correspondingPyTypeObject
'stp_as_number
is nonNULL
, and that should indeed be pretty fast.
You are definitely right
int PyNumber_Check(PyObject *o) { return o && o>ob_type>tp_as_number && (o>ob_type>tp_as_number>nb_int  o>ob_type>tp_as_number>nb_float); }
... (but it answers True
on much more instances than only int/float) ...
Most important is the other question: why should we return x.parent()
if it is not an element?
Vincent
comment:9 in reply to: ↑ 8 Changed 7 years ago by
Replying to vdelecroix:
Most important is the other question: why should we return
x.parent()
if it is not an element?
Because some classes which really should be an Element
aren't actually an Element
. See #17578 for example. If all such bugs are fixed, we could remove that additional test.
comment:10 Changed 7 years ago by
 Commit changed from 3547ea8990d2c2f76001df02dac213956c515e07 to 3a2e193c06f8b26ff18c9555460209a62e54f3db
Branch pushed to git repo; I updated commit sha1. New commits:
3a2e193  Improve documentation

comment:11 Changed 7 years ago by
 Reviewers set to Marc Mezzarobba
 Status changed from needs_review to positive_review
comment:12 Changed 7 years ago by
 Branch changed from u/jdemeyer/ticket/17727 to 3a2e193c06f8b26ff18c9555460209a62e54f3db
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Remove redundant parent_c() functions