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

Save notebook as script using unicode file handle. #1353

Merged
merged 2 commits into from Feb 8, 2012

Conversation

takluyver
Copy link
Member

Using io.open(), which is the Python 3 open() in 2.6 and above.

Closes gh-1345

@ellisonbg
Copy link
Member

A few other places we probably need to track this down:

  • There are other calls to open in the notebookmanager class.
  • In a variety of places in nbformat.v2 and nbformat.v3 (this PR should wait until RST and heading cells #1331 has been merged - it defines the v2 nbformat).

@minrk
Copy link
Member

minrk commented Jan 30, 2012

Actually, this should be fixed at a lower level. json.dumps, which is used by the other writes returns str. The writes method for python scripts should also return str by using py3compat.unicode_to_str so the two (and all future) writes methods match in their return type.

@takluyver
Copy link
Member Author

Actually, I think this way is better than unicode_to_str. On Python 3, that will be a no-op, and it will be encoded by the text-mode file handle, which has a platform dependent default encoding (docs). But we hardcode the encoding comment to utf-8, so we should ensure that we're always encoding with utf-8.

The alternative would be to open a binary-mode file handle and encode it ourselves, but that lacks elegance. Conceptually, Python code is text, so it makes sense to write text.

There are probably other places that would benefit from using io.open - I wasn't aware of it when I wrote py3compat.

@minrk
Copy link
Member

minrk commented Jan 30, 2012

That's a fair point, but we should ensure that both the writes methods do return the same type, which is the real bug here. One or the other should change.

@ellisonbg
Copy link
Member

I am fine with this approach - let's just make sure the other calls to open in the notebook writing code get updated as well.

minrk added a commit that referenced this pull request Feb 8, 2012
Save notebook as script using unicode file handle.

Using io.open(), which is the Python 3 open() in 2.6 and above.

Closes #1345
@minrk minrk merged commit 8e8b2a9 into ipython:master Feb 8, 2012
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Save notebook as script using unicode file handle.

Using io.open(), which is the Python 3 open() in 2.6 and above.

Closes ipython#1345
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.

notebook can't save unicode as script
3 participants