Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finite Family: sort dictionary keys in repr #24438

Closed
embray opened this issue Dec 28, 2017 · 19 comments
Closed

Finite Family: sort dictionary keys in repr #24438

embray opened this issue Dec 28, 2017 · 19 comments

Comments

@embray
Copy link
Contributor

embray commented Dec 28, 2017

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.

Component: misc

Author: Erik Bray

Branch/Commit: u/embray/sets/sort-finite-family-repr @ 4573574

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

@embray embray added this to the sage-8.2 milestone Dec 28, 2017
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 28, 2017

Commit: cf4855e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 28, 2017

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

@embray embray modified the milestones: sage-8.2, sage-8.3 Apr 26, 2018
@tscrim
Copy link
Collaborator

tscrim commented Apr 26, 2018

comment:4

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).

@embray
Copy link
Contributor Author

embray commented Apr 28, 2018

comment:5

I see, that makes sense.

@embray
Copy link
Contributor Author

embray commented May 15, 2018

comment:6

Ah, I knew I already did this, and forgot it still needed work :)
I was just about to do it again...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2018

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,

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2018

Changed commit from cf4855e to 86aaf43

@embray
Copy link
Contributor Author

embray commented May 15, 2018

comment:9

Hold on, just realized this is actually wrong

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2018

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,

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2018

Changed commit from 86aaf43 to 4573574

@tscrim
Copy link
Collaborator

tscrim commented May 15, 2018

comment:12

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.

@tscrim
Copy link
Collaborator

tscrim commented May 16, 2018

comment:13

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.)

@embray
Copy link
Contributor Author

embray commented May 16, 2018

comment:14

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...

@embray
Copy link
Contributor Author

embray commented Jun 27, 2018

comment:15

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

@embray
Copy link
Contributor Author

embray commented Jul 18, 2018

comment:16

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

@embray embray modified the milestones: sage-8.3, sage-8.4 Jul 18, 2018
@tscrim
Copy link
Collaborator

tscrim commented Aug 23, 2018

comment:17

Any progress?

@fchapoton
Copy link
Contributor

comment:18

ticket was duplicated in #26225...

@fchapoton
Copy link
Contributor

comment:19

let us close this one as duplicate

@fchapoton fchapoton removed this from the sage-8.4 milestone Sep 11, 2018
@embray
Copy link
Contributor Author

embray commented Sep 11, 2018

comment:20

Ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants