Skip to content

MNT: Minor cleanups in the pdf backend and related files - #4603

Merged
tacaswell merged 4 commits into
matplotlib:masterfrom
jkseppan:pdf-cleanup
Jul 8, 2015
Merged

tacaswell merged 4 commits into
matplotlib:masterfrom
jkseppan:pdf-cleanup

Conversation

@jkseppan

@jkseppan jkseppan commented Jul 8, 2015

Copy link
Copy Markdown
Member

No description provided.

jkseppan added 4 commits July 8, 2015 15:38
The sole non-ASCII character (in a docstring) had already been
converted to UTF-8 by an earlier commit.
- replace the single use of six.moves.map with a list comprehension
- io.BytesIO is included in all versions of Python we support
- the set type is a builtin in all versions of Python we support
Probably too annoying even for debug-annoying

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://pythonhosted.org/six/ six.BytesIO is StringIO.StringIO in Python 2 not io.BytesIO does that make any difference?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the io.BytesIO version is closer to the corresponding code in Python 3, and six uses StringIO probably because BytesIO was only introduced in 2.6. For example, BytesIO has a readinto method that StringIO lacks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some googling I think you are right.

@jenshnielsen

Copy link
Copy Markdown
Member

One inline comment. The rest is definite improvements IMHO 👍

tacaswell added a commit that referenced this pull request Jul 8, 2015
MNT: Minor cleanups in the pdf backend and related files
@tacaswell
tacaswell merged commit 545c2a0 into matplotlib:master Jul 8, 2015
@jkseppan
jkseppan deleted the pdf-cleanup branch July 8, 2015 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants