ElementWrapperCheckWrappedClass does not implement comparison properly

[jdemeyer]

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

Component: categories

Author: Jeroen Demeyer

Branch/Commit: 8418f8e

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/26890

[jdemeyer]

Dependencies: #26586

[jdemeyer]

Branch: u/jdemeyer/elementwrappercheckwrappedclass_does_not_implement_comparison_properly

[jdemeyer]

New commits:

dc77918	Mark doctest as known bug
8acc586	Delegate comparisons for ElementWrapperCheckWrappedClass to wrapped class

[jdemeyer]

Commit: 8acc586

[jdemeyer]

Author: Jeroen Demeyer

[tscrim]

comment:5

You are changing the comparison logic a bit, in particular, comparisons between two wrapped classes no longer takes into account the parents. Since this is an extension class, it will not get subclassed when constructing elements; you've added an isinstance check that would also take care of if that happened as well. So we would get comparisons between objects that should not necessarily be treated as the same.

For a more concrete example, consider (1,0) as a Z-vector and (1,0) as a partition in an 2x2 box. These are (mathematically) incomparable objects, but your change would have them be equal. So at least the parent check should be added back in.

My recollection of why this was done this way was because we did not want the wrapped objects to have an ordering on them. However, I do not think there is any harm in inducing an ordering from the underlying objects. So the rest of your changes are fine with me.

[sagetrac-git]

Changed commit from 8acc586 to ada6174

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ada6174

Delegate comparisons for ElementWrapperCheckWrappedClass to wrapped class

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

7c515f2

Delegate comparisons for ElementWrapperCheckWrappedClass to wrapped class

[sagetrac-git]

Changed commit from ada6174 to 7c515f2

[tscrim]

comment:10

There is one failure because of the fact that coercion is now taken into account. So we also need this change in sage/combinat/root_system/extended_affine_weyl_group.py:

     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)
         sage: v = W0v.from_reduced_word([1,2])
         sage: x = PvW0(v); x
         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
+

Once changed, you can set a positive review on my behalf.

[tscrim]

Reviewer: Travis Scrimshaw

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8418f8e

Delegate comparisons for ElementWrapperCheckWrappedClass to wrapped class

[sagetrac-git]

Changed commit from 7c515f2 to 8418f8e

[jdemeyer]

Changed dependencies from #26586 to none

[vbraun]

Changed branch from u/jdemeyer/elementwrappercheckwrappedclass_does_not_implement_comparison_properly to 8418f8e

[embray]

comment:15

This tickets were closed as fixed after the Sage 8.5 release.