Output pdf dicts in deterministic order #6427

Merged
merged 2 commits into from May 16, 2016

Conversation

Projects
None yet
4 participants
Member

jkseppan commented May 15, 2016

For #6317 (but not a complete solution for that)

mdboom added the needs_review label May 15, 2016

Owner

tacaswell commented May 16, 2016

Should we merge this as is or do you intend to add more commits to this?

tacaswell added this to the 2.1 (next point release) milestone May 16, 2016

@QuLogic QuLogic and 1 other commented on an outdated diff May 16, 2016

lib/matplotlib/backends/backend_pdf.py
@@ -182,8 +183,8 @@ def pdfRepr(obj):
# represented as Name objects.
elif isinstance(obj, dict):
r = [b"<<"]
- r.extend([Name(key).pdfRepr() + b" " + pdfRepr(val)
- for key, val in six.iteritems(obj)])
+ r.extend([Name(key).pdfRepr() + b" " + pdfRepr(obj[key])
+ for key in sorted(six.iterkeys(obj))])
@QuLogic

QuLogic May 16, 2016

Member

The iterkeys seems unnecessary if you're going to sort it anyway; sorted(obj) should work just fine.

@jkseppan

jkseppan May 16, 2016

Member

Thanks, that makes sense.

@jkseppan jkseppan Make backend_pdf.Name comparable and hashable
9dfb35d
Member

jkseppan commented May 16, 2016

@tacaswell I think it's probably better to merge sooner, it's going to be some work to implement the rest and this could make it easier for someone else to get started.

@tacaswell tacaswell merged commit 0dff252 into matplotlib:master May 16, 2016

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.006%) to 69.736%
Details

tacaswell removed the needs_review label May 16, 2016

jkseppan deleted the jkseppan:pdf-dict-order branch Aug 25, 2016

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