Opened 3 years ago

Closed 2 years ago

#24438 closed enhancement (duplicate)

Finite Family: sort dictionary keys in repr

Reported by: embray Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: misc Keywords:
Cc: Merged in:
Authors: Erik Bray Reviewers:
Report Upstream: N/A Work issues:
Branch: u/embray/sets/sort-finite-family-repr (Commits) Commit: 4573574903119bd5acb682579ee8b6d345214e25
Dependencies: Stopgaps:

Description

Although the order of indices is not necessarily meaningful, it often makes for nicer, more human-meaningful 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 user-friendly, 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 git

  • Commit set to cf4855ef39567cc7a7d28992c70d812a7512c947

Branch pushed to git repo; I updated commit sha1. New commits:

05b00eeFix the FiniteFamily repr to sort its defining dictionary.
cf4855eTests whose expected output needs to be updated now that FiniteFamily's

comment:2 Changed 3 years ago by embray

  • Status changed from new to needs_review

comment:3 Changed 3 years ago by embray

  • Milestone changed from sage-8.2 to sage-8.3

comment:4 Changed 3 years ago by tscrim

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 embray

  • Status changed from needs_review to needs_work

I see, that makes sense.

comment:6 Changed 3 years ago by embray

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 git

  • Commit changed from cf4855ef39567cc7a7d28992c70d812a7512c947 to 86aaf43f48c53b6a1a4fb1de5d6f423bad544104

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

5cea4bbFix the FiniteFamily repr to sort its defining dictionary.
87c7339Tests whose expected output needs to be updated now that FiniteFamily's
86aaf43Change the repr formatting to ensure that if the keys argment was given,

comment:8 Changed 3 years ago by embray

  • Status changed from needs_work to needs_review

comment:9 Changed 3 years ago by embray

  • Status changed from needs_review to needs_work

Hold on, just realized this is actually wrong

comment:10 Changed 3 years ago by git

  • Commit changed from 86aaf43f48c53b6a1a4fb1de5d6f423bad544104 to 4573574903119bd5acb682579ee8b6d345214e25

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

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

comment:11 Changed 3 years ago by embray

  • Status changed from needs_work to needs_review

comment:12 Changed 3 years ago by tscrim

What if the keys are not sortable, say 'a' and 1 (in Python3)? I think the sorting order needs to be in a try-except block and just default to some ordering if sorted fails.

comment:13 Changed 3 years ago by tscrim

I think this will fail with your branch:

sage: sorted([complex(1), complex(2)])
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-7-6dddb83c526f> 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 embray

  • Status changed from needs_review to needs_work

True--I 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 2-style ordering for unorderable types.

I mentioned in my commit message that we might want to add something like this to Sage--I'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 2 years ago by embray

Still planning to work on fixing this up. I'm running into issues with these again :)

comment:16 Changed 2 years ago by embray

  • Milestone changed from sage-8.3 to sage-8.4

I believe this issue can reasonably be addressed for Sage 8.4.

comment:17 Changed 2 years ago by tscrim

Any progress?

comment:18 Changed 2 years ago by chapoton

ticket was duplicated in #26225...

comment:19 Changed 2 years ago by chapoton

  • Milestone changed from sage-8.4 to sage-duplicate/invalid/wontfix
  • Status changed from needs_work to needs_review

let us close this one as duplicate

comment:20 Changed 2 years ago by embray

  • Resolution set to duplicate
  • Status changed from needs_review to closed

Ok.

Note: See TracTickets for help on using tickets.