#24759 closed enhancement (fixed)

Sort the repr of PolyDict by its dict keys

Reported by: embray Owned by:
Priority: minor Milestone: sage-8.2
Component: misc Keywords:
Cc: Merged in:
Authors: Erik Bray Reviewers: Jeroen Demeyer, Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: f2ea845 (Commits) Commit: f2ea84552df2c912b25fd5d37ddd4bcdc493733d
Dependencies: Stopgaps:

Description

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

Change History (11)

comment:1 Changed 22 months ago by embray

  • Status changed from new to needs_review

comment:2 Changed 22 months ago by git

  • Commit changed from ad72d11978aa04047bb430c4c1743550e78a3ce9 to 8596fe88839e1841850842cf06885708219b6d04

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

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

comment:3 Changed 22 months ago by jdemeyer

  • Reviewers set to Jeroen Demeyer

Interesting. I never heard about pprint.

positive review if tests pass.

comment:4 Changed 22 months ago by jdemeyer

  • Status changed from needs_review to needs_work

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

comment:5 Changed 22 months ago by chapoton

  • Branch changed from u/embray/polydict-sort-repr to public/polydict-sort-repr
  • Commit changed from 8596fe88839e1841850842cf06885708219b6d04 to 43d923459ef3b4adc0235d93e32cf1f0047060e5
  • Status changed from needs_work to needs_review

New commits:

43d9234fixing doctests

comment:6 Changed 22 months ago by chapoton

  • Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Frédéric Chapoton
  • Status changed from needs_review to positive_review

green bot. I am setting to positive.

comment:7 Changed 22 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:8 Changed 22 months ago by git

  • Commit changed from 43d923459ef3b4adc0235d93e32cf1f0047060e5 to f2ea84552df2c912b25fd5d37ddd4bcdc493733d

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

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

comment:9 Changed 22 months ago by chapoton

  • Status changed from needs_work to positive_review

comment:10 Changed 22 months ago by embray

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

comment:11 Changed 22 months ago by vbraun

  • Branch changed from public/polydict-sort-repr to f2ea84552df2c912b25fd5d37ddd4bcdc493733d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.