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 filename to open in notebookapp.py after #4260 and #4265 #4340

Closed
wants to merge 1 commit into from
Closed

Fix filename to open in notebookapp.py after #4260 and #4265 #4340

wants to merge 1 commit into from

Conversation

slel
Copy link

@slel slel commented Jan 11, 2019

This also needs to be backported to the 5.7.x branch and should fix #4303.

@takluyver
Copy link
Member

That wasn't a mistake, open() on Python 3 takes a file descriptor, and closes it when it's done.

We should use io.open() for Python 2, which is effectively the Python 3 open() function, and accepts a file descriptor. Can you submit a PR to do this against the 5.7.x branch (master has already dropped Python 2 support, so we don't need any change there).

@slel
Copy link
Author

slel commented Jan 16, 2019

Thanks for clarifying. I'll open a PR against the 5.7.x branch
changing with open to with io.open at line 1742 of the file
notebook/notebookapp.py.

What about the occurrence at line 1674, does that one stay as is?

https://github.com/jupyter/notebook/blob/5.7.x/notebook/notebookapp.py#L1742
https://github.com/jupyter/notebook/blob/5.7.x/notebook/notebookapp.py#L1674

@takluyver
Copy link
Member

Thanks! Yes, I think they should both use io.open(..., encoding='utf-8').

@slel
Copy link
Author

slel commented Jan 16, 2019

See #4348.

@slel slel closed this Jan 16, 2019
@slel slel deleted the fix-issue-4303 branch January 31, 2019 22:35
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot start jupyter notebook
2 participants