Navigation Menu

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

fix forgotten cStringIO replacement when replacing 2to3 with six #2385

Closed
wants to merge 1 commit into from
Closed

Conversation

davidovitch
Copy link
Contributor

...x

Error introduced with commit f4adec7 (Use six instead of 2to3). For Python2 cStringIO was still being used, but the import statement was removed. Both Python2 and 3 should use io.StringIO, which is the same as elsewhere in the code.

… six

Error introduced with commit f4adec7 (Use six instead of 2to3). For Python2 cStringIO was still being used, but the import statement was removed. Both Python2 and 3 should use io.StringIO, which is the same as elsewhere in the code.
@mrterry
Copy link
Contributor

mrterry commented Sep 5, 2013

Are io.StringIO and cStringIO.StringIO the same on python 2? Why not use six.moves.cStringIO?

@mdboom
Copy link
Member

mdboom commented Sep 5, 2013

Yeah -- I think it needs to be six.moves.cStringIO -- it needs to be for bytes on Python 2 and unicode on Python 3.

@davidovitch
Copy link
Contributor Author

Thanks for reviewing. I'll close this one and make a new PR but not against the master, but a bug-tree. If I understand it correctly that is the preferred way of working, right?

@efiring
Copy link
Member

efiring commented Sep 8, 2013

@davidovitch I'm not sure I understand your question. The procedure is to make a branch in your clone, starting from current master; make the change in that branch; push that branch to your clone on github; and make the pull request for master to pull your branch. That is usually what is meant by a "PR against master": the github matplotlib master is asked to pull in a branch with a different name from your github repo, where you made that branch relative to current matplotlib master.

@davidovitch
Copy link
Contributor Author

@efiring thanks for clarifying. My previous question was not formulated correctly, but the work flow you describe is what I had in mind. If I understand it correctly, this PR violated the outlined work flow. So that's why I closed it, implemented the comments, and re-opened it in PR #2393 using the correct work flow. Apologies for these beginner mistakes.

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.

None yet

4 participants