#19497: Improve comparison framework
-------------------------------------+-------------------------------------
Reporter: jdemeyer | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.10
Component: cython | Resolution:
Keywords: | Merged in:
Authors: Jeroen Demeyer | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/jdemeyer/improve_comparison_framework|
2497282901353b174fc8d290be0532c3874d4952
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by vdelecroix):
Replying to [comment:9 jdemeyer]:
> Replying to [comment:7 vdelecroix]:
> > These are not equivalent
> > {{{
> > - elif PyInt_CheckExact(right):
> > + elif isinstance(right, int):
> > }}}
> > As far as I understand, `PyInt_Check` is equivalent to `isinstance(.,
int)` and `PyInt_CheckExact` is equivalent to `type(.) is int`. Why did
you change them in this ticket?
>
> First of all, it's cleaner to write pure Python code than using Python
C/API calls (and in this case, it's equally fast). The reason I chose
`isinstance(...)` instead of `type(...) is ...` is simply because the
former is usually the right thing to do. In Python, checking equality of
types is not normally done.
Perhaps cython is smart enough to be as fast as a direct call to the C API
but `PyInt_CheckExact` is faster than `PyInt_Check`... Do we really want
to consider a thing inhering from `int` as an `int` in a comparison? Do
you have a usecase that would help to decide?
For `CoercionModel` it is good as it is.
--
Ticket URL: <http://trac.sagemath.org/ticket/19497#comment:10>
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.
