Skip to content

Commit

Permalink
Trac #26890: ElementWrapperCheckWrappedClass does not implement compa…
Browse files Browse the repository at this point in the history
…rison properly

Comparisons with the wrapped class should just delegate to the wrapped
class, handling also inequalities instead of only `==` and `!=`.

URL: https://trac.sagemath.org/26890
Reported by: jdemeyer
Ticket author(s): Jeroen Demeyer
Reviewer(s): Travis Scrimshaw
  • Loading branch information
Release Manager authored and vbraun committed Dec 23, 2018
2 parents 6c40fa6 + 8418f8e commit 110bcf7
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 40 deletions.
2 changes: 1 addition & 1 deletion src/sage/categories/commutative_rings.py
Expand Up @@ -208,7 +208,7 @@ def cyclotomic_cosets(self, q, cosets=None):
sage: R = cartesian_product([GF(7), Zmod(14)])
sage: a = R((3,5))
sage: R.cyclotomic_cosets((3,5), [(1,1)])
[[(1, 1), (3, 5), (2, 11), (6, 13), (4, 9), (5, 3)]]
[[(1, 1), (2, 11), (3, 5), (4, 9), (5, 3), (6, 13)]]
"""
q = self(q)

Expand Down
8 changes: 4 additions & 4 deletions src/sage/combinat/posets/poset_examples.py
Expand Up @@ -639,11 +639,11 @@ def ProductOfChains(chain_lengths, facade=None):
sage: P = posets.ProductOfChains([2, 2]); P
Finite lattice containing 4 elements
sage: P.linear_extension()
[(0, 0), (1, 0), (0, 1), (1, 1)]
[(0, 0), (0, 1), (1, 0), (1, 1)]
sage: P.upper_covers((0,0))
[(1, 0), (0, 1)]
[(0, 1), (1, 0)]
sage: P.lower_covers((1,1))
[(1, 0), (0, 1)]
[(0, 1), (1, 0)]
TESTS::
Expand Down Expand Up @@ -1397,7 +1397,7 @@ def YoungDiagramPoset(lam):
sage: P = posets.YoungDiagramPoset(Partition([2,2])); P
Finite meet-semilattice containing 4 elements
sage: sorted(P.cover_relations()) # known bug (Trac #26890)
sage: sorted(P.cover_relations())
[[(0, 0), (0, 1)], [(0, 0), (1, 0)], [(0, 1), (1, 1)], [(1, 0), (1, 1)]]
"""
def cell_leq(a, b):
Expand Down
13 changes: 9 additions & 4 deletions src/sage/combinat/root_system/extended_affine_weyl_group.py
Expand Up @@ -303,8 +303,9 @@ def ExtendedAffineWeylGroup(cartan_type, general_linear=None, **print_options):
sage: PW0(w)
t[Lambdacheck[1] - 2*Lambdacheck[2]] * s1
Note that for untwisted affine type the dual form of the classical Weyl group
is isomorphic to the usual one, but acts on a different lattice and is therefore different to sage::
Note that for untwisted affine type, the dual form of the classical
Weyl group is isomorphic to the usual one, but acts on a different
lattice and is therefore different to sage::
sage: W0v = E.dual_classical_weyl(); W0v
Weyl Group of type ['A', 2] (as a matrix group acting on the weight lattice)
Expand All @@ -313,11 +314,15 @@ def ExtendedAffineWeylGroup(cartan_type, general_linear=None, **print_options):
s1*s2
sage: y = PW0(v); y
s1*s2
sage: x == y
False
sage: x.parent() == y.parent()
False
However, because there is a coercion from ``PvW0`` to ``PW0``,
the elements ``x`` and ``y`` compare as equal::
sage: x == y
True
An element can be created directly from a reduced word::
sage: PW0.from_reduced_word([2,1,0])
Expand Down
67 changes: 36 additions & 31 deletions src/sage/structure/element_wrapper.pyx
Expand Up @@ -19,12 +19,14 @@ AUTHORS:
# https://www.gnu.org/licenses/
# ****************************************************************************

from cpython.object cimport Py_EQ, Py_NE, Py_LE, Py_GE
from cpython.object cimport Py_EQ, Py_NE, Py_LE, Py_GE, PyObject_RichCompare

from sage.structure.parent cimport Parent
from sage.structure.element cimport Element, coercion_model
from sage.structure.unique_representation import UniqueRepresentation
from copy import copy


cdef class ElementWrapper(Element):
r"""
A class for wrapping Sage or Python objects as Sage elements.
Expand Down Expand Up @@ -404,8 +406,7 @@ cdef class ElementWrapper(Element):
res.value = copy(self.value)
return res

from sage.structure.parent import Parent
from sage.structure.unique_representation import UniqueRepresentation

class DummyParent(UniqueRepresentation, Parent):
"""
A class for creating dummy parents for testing ElementWrapper
Expand Down Expand Up @@ -521,41 +522,45 @@ cdef class ElementWrapperCheckWrappedClass(ElementWrapper):
"""
return hash(self.value)

def __richcmp__(left, right, int op):
def __richcmp__(self, right, int op):
"""
Return ``True`` if ``left`` compares with ``right`` based on ``op``.
Return ``True`` if ``self`` compares with ``right`` based on ``op``.
.. SEEALSO::
:meth:`ElementWrapper.__richcmp__`
TESTS::
EXAMPLES::
sage: A = cartesian_product([ZZ, ZZ])
sage: elt = A((1,1))
sage: (1, 1) == elt
True
sage: elt == (1, 1)
True
sage: A((1, 2)) == elt
False
"""
cdef ElementWrapperCheckWrappedClass self
self = left

if self.__class__ != right.__class__:
if isinstance(right, self.wrapped_class):
if op == Py_EQ or op == Py_LE or op == Py_GE:
return self.value == right
if op == Py_NE:
return self.value != right
return False
return op == Py_NE
if self._parent != (<ElementWrapper>right)._parent:
return op == Py_NE
if op == Py_EQ or op == Py_LE or op == Py_GE:
return self.value == (<ElementWrapper>right).value
if op == Py_NE:
return self.value != (<ElementWrapper>right).value
return False
sage: A((1, 2)) > elt
True
sage: elts = [A((x,y)) for y in range(3) for x in range(2)]
sage: elts
[(0, 0), (1, 0), (0, 1), (1, 1), (0, 2), (1, 2)]
sage: sorted(elts)
[(0, 0), (0, 1), (0, 2), (1, 0), (1, 1), (1, 2)]
::
sage: A = cartesian_product([ZZ, ZZ])
sage: B = cartesian_product([GF(3), GF(5)])
sage: A((3,5)) == B((0,0))
True
"""
if type(self) is type(right):
# Both are instances of ElementWrapperCheckWrappedClass:
# compare using wrapped element if the parents are the same
other = <ElementWrapperCheckWrappedClass>right
if self._parent is other._parent:
return PyObject_RichCompare(self.value, other.value, op)
elif not isinstance(right, Element):
# Right is not an Element: compare using wrapped element
return PyObject_RichCompare(self.value, right, op)
elif self._parent is (<Element>right)._parent:
# Different types but same parent? This should not happen
raise TypeError(f"cannot compare {type(self).__name__} with {type(right).__name__} if parents are equal")

# Different parents => use coercion model
return coercion_model.richcmp(self, right, op)

0 comments on commit 110bcf7

Please sign in to comment.