IncidenceStructure breaks if points cannot be sorted

[jdemeyer]

This sort can fail if the points are not comparable:

                sortkey = lambda e: [(0 if x is None else 2 if isinstance(x, str) else 1, x) for x in e]\
                    if isinstance(e, tuple) else e
                self._points = sorted(points, key=sortkey)

The sortkey tries to find a clever way to "sort" the points but hardcoding classes like that is not a general solution. In particular, it's still comparing integers and strings when #22029 is appplied.

Instead, just don't sort.

CC: @vinklein

Component: combinatorics

Author: Jeroen Demeyer

Branch/Commit: a354dc1

Reviewer: Frédéric Chapoton

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

[vinklein]

comment:4

A mix of points of strings type and integers type will not make this function fail.

This work in python3:

>>> (1, 3) < (2, 'a string')
True

The comparison stops if the first element of each tuple are different.

[vinklein]

comment:5

Also, the sortkey should be implemented as ordinary function def sortkey instead of this unreadable one-liner (why do people feel that a lambda is required to define a sorting key?).

Indeed, pep8 says so, my bad.

[jdemeyer]

comment:6

Replying to @vinklein:

A mix of points of strings type and integers type will not make this function fail.

Apparently, it does. See the patchbot report on #22029. I have not looked at the details.

[jdemeyer]

comment:7

On #22029, I do get

TypeError: unsupported operand parent(s) for >: 'Integer Ring' and '<type 'str'>'

and it's pointing to the line

self._points = sorted(points, key=sortkey)

[jdemeyer]

Branch: u/jdemeyer/incidencestructure_breaks_if_points_cannot_be_sorted

[jdemeyer]

Author: Jeroen Demeyer

[jdemeyer]

Commit: a354dc1

[jdemeyer]

New commits:

a354dc1

Do not sort points in IncidenceStructure

[fchapoton]

comment:11

ok, let it be

[fchapoton]

Reviewer: Frédéric Chapoton

[embray]

comment:12

Retarging tickets optimistically to the next milestone. If you are responsible for this ticket (either its reporter or owner) and don't believe you are likely to complete this ticket before the next release (8.7) please retarget this ticket's milestone to sage-pending or sage-wishlist.

[vbraun]

Changed branch from u/jdemeyer/incidencestructure_breaks_if_points_cannot_be_sorted to a354dc1