Opened 3 years ago
Closed 3 years ago
#24438 closed enhancement (duplicate)
Finite Family: sort dictionary keys in repr
Reported by:  embray  Owned by:  

Priority:  major  Milestone:  sageduplicate/invalid/wontfix 
Component:  misc  Keywords:  
Cc:  Merged in:  
Authors:  Erik Bray  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  u/embray/sets/sortfinitefamilyrepr (Commits, GitHub, GitLab)  Commit:  4573574903119bd5acb682579ee8b6d345214e25 
Dependencies:  Stopgaps: 
Description
Although the order of indices is not necessarily meaningful, it often makes for nicer, more humanmeaningful output to sort them. For example:
sage: A = Algebras(QQ).WithBasis().Filtered().example() sage: grA = A.graded_algebra() sage: grA.algebra_generators() Finite family {'x': bar(U['x']), 'y': bar(U['y']), 'z': bar(U['z'])
instead of the more seemingly arbitrary
Finite family {'y': bar(U['y']), 'x': bar(U['x']), 'z': bar(U['z'])
which of course is itself a result that is not in any way guaranteed.
In addition to being more userfriendly, this change will also help a lot with tests that otherwise fail on Python 3 due to arbitrary differences in dict key ordering.
Change History (20)
comment:1 Changed 3 years ago by
 Commit set to cf4855ef39567cc7a7d28992c70d812a7512c947
comment:2 Changed 3 years ago by
 Status changed from new to needs_review
comment:3 Changed 3 years ago by
 Milestone changed from sage8.2 to sage8.3
comment:4 Changed 3 years ago by
I had meant to respond to this earlier, but it fell off my radar.
I think that FiniteFamily
should print out following self._keys
if that is not None
(which also dictates iteration order) and then fallback to the sorted output (see the values
method).
comment:5 Changed 3 years ago by
 Status changed from needs_review to needs_work
I see, that makes sense.
comment:6 Changed 3 years ago by
Ah, I knew I already did this, and forgot it still needed work :) I was just about to do it again...
comment:7 Changed 3 years ago by
 Commit changed from cf4855ef39567cc7a7d28992c70d812a7512c947 to 86aaf43f48c53b6a1a4fb1de5d6f423bad544104
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5cea4bb  Fix the FiniteFamily repr to sort its defining dictionary.

87c7339  Tests whose expected output needs to be updated now that FiniteFamily's

86aaf43  Change the repr formatting to ensure that if the keys argment was given,

comment:8 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:9 Changed 3 years ago by
 Status changed from needs_review to needs_work
Hold on, just realized this is actually wrong
comment:10 Changed 3 years ago by
 Commit changed from 86aaf43f48c53b6a1a4fb1de5d6f423bad544104 to 4573574903119bd5acb682579ee8b6d345214e25
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4573574  Change the repr formatting to ensure that if the keys argment was given,

comment:11 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:12 Changed 3 years ago by
What if the keys are not sortable, say 'a'
and 1
(in Python3)? I think the sorting order needs to be in a tryexcept block and just default to some ordering if sorted
fails.
comment:13 Changed 3 years ago by
I think this will fail with your branch:
sage: sorted([complex(1), complex(2)])  TypeError Traceback (most recent call last) <ipythoninput76dddb83c526f> in <module>() > 1 sorted([complex(Integer(1)), complex(Integer(2))]) TypeError: no ordering relation is defined for complex numbers sage: L = [complex(1), complex(2)] sage: Family(L, lambda x: x) Finite family {(1+0j): (1+0j), (2+0j): (2+0j)}
(I am at the airport waiting for a flight and my laptop only have 8.3.beta0, so I cannot test it right now.)
comment:14 Changed 3 years ago by
 Status changed from needs_review to needs_work
TrueI didn't see any cases in the tests where that was a problem but that could certainly happen I guess. On Python 3 pprint
uses a "safe key" wrapper for the keys which falls back to Python 2style ordering for unorderable types.
I mentioned in my commit message that we might want to add something like this to SageI've definitely been finding other places in Sage where one would like to at least order things as best possible. Maybe I'll go ahead and do that first...
comment:15 Changed 3 years ago by
Still planning to work on fixing this up. I'm running into issues with these again :)
comment:16 Changed 3 years ago by
 Milestone changed from sage8.3 to sage8.4
I believe this issue can reasonably be addressed for Sage 8.4.
comment:17 Changed 3 years ago by
Any progress?
comment:18 Changed 3 years ago by
ticket was duplicated in #26225...
comment:19 Changed 3 years ago by
 Milestone changed from sage8.4 to sageduplicate/invalid/wontfix
 Status changed from needs_work to needs_review
let us close this one as duplicate
comment:20 Changed 3 years ago by
 Resolution set to duplicate
 Status changed from needs_review to closed
Ok.
Branch pushed to git repo; I updated commit sha1. New commits:
Fix the FiniteFamily repr to sort its defining dictionary.
Tests whose expected output needs to be updated now that FiniteFamily's