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

Sort the repr of PolyDict by its dict keys #24759

Closed
embray opened this issue Feb 16, 2018 · 15 comments
Closed

Sort the repr of PolyDict by its dict keys #24759

embray opened this issue Feb 16, 2018 · 15 comments

Comments

@embray
Copy link
Contributor

embray commented Feb 16, 2018

This is one of those cases where a class has a dict representation embedded in its repr, so where IPython would normally pprint the plain dict it does not pprint the embedded dict.

The PolyDict class is only used internally so this should be a low impact change, but that enhances its testability (especially in moving to Python 3).

Component: misc

Author: Erik Bray

Branch/Commit: f2ea845

Reviewer: Jeroen Demeyer, Frédéric Chapoton

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

@embray embray added this to the sage-8.2 milestone Feb 16, 2018
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 16, 2018

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

8596fe8Sort the PolyDict keys (lexicographically) in its repr.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 16, 2018

Changed commit from ad72d11 to 8596fe8

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@jdemeyer
Copy link

comment:3

Interesting. I never heard about pprint.

positive review if tests pass.

@jdemeyer
Copy link

comment:4

Failing tests in src/sage/rings/polynomial/polydict.pyx

@fchapoton
Copy link
Contributor

Changed branch from u/embray/polydict-sort-repr to public/polydict-sort-repr

@fchapoton
Copy link
Contributor

New commits:

43d9234fixing doctests

@fchapoton
Copy link
Contributor

Changed commit from 8596fe8 to 43d9234

@fchapoton
Copy link
Contributor

Changed reviewer from Jeroen Demeyer to Jeroen Demeyer, Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:6

green bot. I am setting to positive.

@vbraun
Copy link
Member

vbraun commented Feb 18, 2018

comment:7

Merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 19, 2018

Changed commit from 43d9234 to f2ea845

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 19, 2018

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

f2ea845Merge branch 'public/polydict-sort-repr' in 8.2.b6

@embray
Copy link
Contributor Author

embray commented Feb 19, 2018

comment:10

Huh, thanks for fixing those additional tests. Those must be new since I first made this change (which was actually quite a few weeks ago now).

@vbraun
Copy link
Member

vbraun commented Feb 20, 2018

Changed branch from public/polydict-sort-repr to f2ea845

@vbraun vbraun closed this as completed in 6ad7c8a Feb 20, 2018
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

4 participants