Opened 17 months ago
Closed 16 months ago
#26938 closed defect (fixed)
IncidenceStructure breaks if points cannot be sorted
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage8.7 
Component:  combinatorics  Keywords:  
Cc:  vklein  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  a354dc1 (Commits)  Commit:  a354dc129b0085acf88b4b1cbae1d5029218fa61 
Dependencies:  Stopgaps: 
Description (last modified by )
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.
Change History (13)
comment:1 Changed 17 months ago by
 Description modified (diff)
comment:2 Changed 17 months ago by
 Description modified (diff)
comment:3 Changed 17 months ago by
 Description modified (diff)
comment:4 followup: ↓ 6 Changed 17 months ago by
comment:5 Changed 17 months ago by
Also, the sortkey should be implemented as ordinary function def sortkey instead of this unreadable oneliner (why do people feel that a lambda is required to define a sorting key?).
Indeed, pep8 says so, my bad.
comment:6 in reply to: ↑ 4 Changed 17 months ago by
comment:7 Changed 17 months ago by
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)
comment:8 Changed 17 months ago by
 Branch set to u/jdemeyer/incidencestructure_breaks_if_points_cannot_be_sorted
comment:9 Changed 17 months ago by
 Commit set to a354dc129b0085acf88b4b1cbae1d5029218fa61
 Description modified (diff)
 Status changed from new to needs_review
New commits:
a354dc1  Do not sort points in IncidenceStructure

comment:10 Changed 17 months ago by
 Description modified (diff)
comment:11 Changed 17 months ago by
 Reviewers set to Frédéric Chapoton
 Status changed from needs_review to positive_review
ok, let it be
comment:12 Changed 17 months ago by
 Milestone changed from sage8.6 to sage8.7
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 sagepending or sagewishlist.
comment:13 Changed 16 months ago by
 Branch changed from u/jdemeyer/incidencestructure_breaks_if_points_cannot_be_sorted to a354dc129b0085acf88b4b1cbae1d5029218fa61
 Resolution set to fixed
 Status changed from positive_review to closed
A mix of points of strings type and integers type will not make this function fail.
This work in python3:
The comparison stops if the first element of each tuple are different.