Skip to content

Commit

Permalink
Trac #19208: Replace remaining instances of has_coerce_map_from_[c_]i…
Browse files Browse the repository at this point in the history
…mpl by _coerce_map_from_

The two remaining occurrences of the old-style
`has_coerce_map_from_[c_]impl` should be replaced by
`_coerce_map_from_()`.

In addition, `Singular._coerce_map_from_()` now returns `None` if no
coercion map is known (instead of returning `False` or raising a
`NotImplementedError`).

We remove some left-overs of the old coercion model in `homset.py`
(which uses the new coercion model now).

We also remove completely the methods `_coerce_self_c`, `_coerce_self`,
`has_coerce_map_from_impl`, `coerce_map_from_impl`.

URL: http://trac.sagemath.org/19208
Reported by: pbruin
Ticket author(s): Peter Bruin, Jeroen Demeyer
Reviewer(s): Jeroen Demeyer, Peter Bruin
  • Loading branch information
Release Manager authored and vbraun committed Sep 19, 2015
2 parents aaaa782 + c3b1e6e commit 4cc6f80
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 89 deletions.
3 changes: 0 additions & 3 deletions src/doc/en/thematic_tutorials/coercion_and_categories.rst
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ This base class provides a lot more methods than a general parent::
'_coerce_',
'_coerce_c',
'_coerce_impl',
'_coerce_self',
'_coerce_try',
'_default_category',
'_gcd_univariate_polynomial',
Expand All @@ -140,7 +139,6 @@ This base class provides a lot more methods than a general parent::
'cardinality',
'class_group',
'coerce_map_from_c',
'coerce_map_from_impl',
'content',
'divides',
'epsilon',
Expand All @@ -153,7 +151,6 @@ This base class provides a lot more methods than a general parent::
'get_action_c',
'get_action_impl',
'has_coerce_map_from_c',
'has_coerce_map_from_impl',
'ideal',
'ideal_monoid',
'integral_closure',
Expand Down
27 changes: 0 additions & 27 deletions src/sage/categories/homset.py
Original file line number Diff line number Diff line change
Expand Up @@ -1185,33 +1185,6 @@ def reversed(self):
"""
return Hom(self.codomain(), self.domain(), category = self.homset_category())

############### For compatibility with old coercion model #######################

def get_action_c(self, R, op, self_on_left):
"""
.. WARNING::
For compatibility with old coercion model. DO NOT USE!
TESTS::
sage: H = Hom(ZZ^2, ZZ^3)
sage: H.get_action_c(ZZ, operator.add, ZZ)
"""
return None

def coerce_map_from_c(self, R):
"""
.. WARNING::
For compatibility with old coercion model. DO NOT USE!
TESTS::
sage: H = Hom(ZZ^2, ZZ^3)
sage: H.coerce_map_from_c(ZZ)
"""
return None

# Really needed???
class HomsetWithBase(Homset):
Expand Down
2 changes: 1 addition & 1 deletion src/sage/interfaces/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ def __getattr__(self, attrname):
TESTS::
sage: ParentWithBase.__getattribute__(singular, '_coerce_map_from_')
<built-in method _coerce_map_from_ of Singular object at ...>
<bound method Singular._coerce_map_from_ of Singular>
"""
try:
return ParentWithBase.__getattribute__(self, attrname)
Expand Down
23 changes: 18 additions & 5 deletions src/sage/interfaces/singular.py
Original file line number Diff line number Diff line change
Expand Up @@ -785,20 +785,33 @@ def __call__(self, x, type='def'):

return SingularElement(self, type, x, False)

def has_coerce_map_from_impl(self, S):
def _coerce_map_from_(self, S):
"""
Return ``True`` if ``S`` admits a coercion map into the
Singular interface.
EXAMPLES::
sage: singular._coerce_map_from_(ZZ)
True
sage: singular.coerce_map_from(ZZ)
Call morphism:
From: Integer Ring
To: Singular
sage: singular.coerce_map_from(float)
"""
# we want to implement this without coercing, since singular has state.
if hasattr(S, 'an_element'):
if hasattr(S.an_element(), '_singular_'):
return True
try:
self._coerce_(S.an_element())
return True
except TypeError:
return False
return True
pass
elif S is int or S is long:
return True
raise NotImplementedError

return None

def cputime(self, t=None):
r"""
Expand Down
17 changes: 16 additions & 1 deletion src/sage/libs/pari/pari_instance.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,22 @@ cdef class PariInstance(PariInstance_auto):
def __hash__(self):
return 907629390 # hash('pari')

cdef has_coerce_map_from_c_impl(self, x):
cpdef _coerce_map_from_(self, x):
"""
Return ``True`` if ``x`` admits a coercion map into the
PARI interface.
This currently always returns ``True``.
EXAMPLES::
sage: pari._coerce_map_from_(ZZ)
True
sage: pari.coerce_map_from(ZZ)
Call morphism:
From: Integer Ring
To: Interface to the PARI C library
"""
return True

def __richcmp__(left, right, int op):
Expand Down
4 changes: 2 additions & 2 deletions src/sage/modular/modform/space.py
Original file line number Diff line number Diff line change
Expand Up @@ -962,14 +962,14 @@ def _has_natural_inclusion_map_to(self, right):
return f.parent()(e) == f
raise NotImplementedError

def has_coerce_map_from_impl(self, from_par):
def _coerce_map_from_(self, from_par):
"""
Code to make ModularFormsSpace work well with coercion framework.
EXAMPLES::
sage: M = ModularForms(22,2)
sage: M.has_coerce_map_from_impl(M.cuspidal_subspace())
sage: M.has_coerce_map_from(M.cuspidal_subspace())
True
sage: M.has_coerce_map_from(ModularForms(22,4))
False
Expand Down
1 change: 0 additions & 1 deletion src/sage/structure/parent_old.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ cdef class Parent(parent.Parent):
cdef has_coerce_map_from_c_impl(self, S)
cpdef _coerce_c(self, x)
cdef _coerce_c_impl(self, x)
cdef _coerce_self_c(self, x)

cdef _an_element_c_impl(self)
cpdef _an_element_c(self)
52 changes: 3 additions & 49 deletions src/sage/structure/parent_old.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,6 @@ cdef class Parent(parent.Parent):
# New Coercion support functionality
#################################################################################

# def coerce_map_from(self, S):
# return self.coerce_map_from_c(S)

cpdef coerce_map_from_c(self, S):
"""
EXAMPLES:
Expand Down Expand Up @@ -151,18 +148,15 @@ cdef class Parent(parent.Parent):
except KeyError:
pass

if HAS_DICTIONARY(self):
mor = self.coerce_map_from_impl(S)
else:
mor = self.coerce_map_from_c_impl(S)
mor = self.coerce_map_from_c_impl(S)
import sage.categories.morphism
import sage.categories.map
if mor is True:
mor = sage.categories.morphism.CallMorphism(S, self)
elif mor is False:
mor = None
elif mor is not None and not isinstance(mor, sage.categories.map.Map):
raise TypeError, "coerce_map_from_impl must return a boolean, None, or an explicit Map"
raise TypeError("coerce_map_from_c_impl must return a boolean, None, or an explicit Map")

if mor is None and isinstance(S, type):
#Convert Python types to native Sage types
Expand All @@ -179,10 +173,6 @@ cdef class Parent(parent.Parent):

return mor

def coerce_map_from_impl(self, S):
check_old_coerce(self)
return self.coerce_map_from_c_impl(S)

cdef coerce_map_from_c_impl(self, S):
check_old_coerce(self)
import sage.categories.morphism
Expand Down Expand Up @@ -213,9 +203,6 @@ cdef class Parent(parent.Parent):
else:
return None

# def get_action(self, S, op=operator.mul, self_on_left=True):
# return self.get_action_c(S, op, self_on_left)

cpdef get_action_c(self, S, op, bint self_on_left):
check_old_coerce(self)
try:
Expand Down Expand Up @@ -307,30 +294,6 @@ cdef class Parent(parent.Parent):
pass
raise TypeError, "no canonical coercion of element into self"

def _coerce_self(self, x):
check_old_coerce(self)
return self._coerce_self_c(x)

cdef _coerce_self_c(self, x):
"""
Try to canonically coerce x into self.
Return result on success or raise TypeError on failure.
"""
check_old_coerce(self)
# todo -- optimize?
try:
P = x.parent()
if P is self:
return x
elif P == self:
return self(x)
except AttributeError:
pass
raise TypeError, "no canonical coercion to self defined"

# def has_coerce_map_from(self, S):
# return self.has_coerce_map_from_c(S)

cpdef has_coerce_map_from_c(self, S):
"""
Return True if there is a natural map from S to self.
Expand All @@ -346,17 +309,10 @@ cdef class Parent(parent.Parent):
return self._has_coerce_map_from.get(S)
except KeyError:
pass
if HAS_DICTIONARY(self):
x = self.has_coerce_map_from_impl(S)
else:
x = self.has_coerce_map_from_c_impl(S)
x = self.has_coerce_map_from_c_impl(S)
self._has_coerce_map_from.set(S, x)
return x

def has_coerce_map_from_impl(self, S):
check_old_coerce(self)
return self.has_coerce_map_from_c_impl(S)

cdef has_coerce_map_from_c_impl(self, S):
check_old_coerce(self)
if not isinstance(S, parent.Parent):
Expand All @@ -365,8 +321,6 @@ cdef class Parent(parent.Parent):
self._coerce_c((<parent.Parent>S).an_element())
except TypeError:
return False
except NotImplementedError as msg:
raise NotImplementedError, "%s\nAlso, please make sure you have implemented has_coerce_map_from_impl or has_coerce_map_from_c_impl (or better _an_element_c_impl or _an_element_impl if possible) for %s"%(msg,self)
return True

def _an_element_impl(self): # override this in Python
Expand Down

0 comments on commit 4cc6f80

Please sign in to comment.