-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Keep line-endings in ipynb #1663
Conversation
I should note that this will visibly affect people tracking notebooks in VCS, as it will append the newline character to every line of text/code cells after it is applied. |
Well, this is a dev version, and from 0.12 to 0.13 you have a notebook conversion step ... so i don't think it will really be a problem. |
I do not think it is a big deal, just making it clear what is involved. Also, there are definitely people using 0.13 notebooks in real work. |
I think this looks fine. @minrk why don't you merge this unless you think there is something else you want to do. |
I just thought of one more case where this will not be ideal, though I'm not sure we really care about it: it adds one more step to the downgrade-notebook gist, because v3 notebooks read by v2 (after manually decrementing the version) will get double newlines. Shall I re-run the existing notebooks with this change? They will all get a 2-char diff on each line of multi-line cells. |
For the v2/v3 conversion does this just affect output areas where For v3 notebooks before this PR, they will work fine, they just get the additional new line at the end of each line of text? Not sure it is worth the effort to update all the notebooks, as the changes will be made gradually as we re-run those notebooks. But there is no harm in doing it if you want to. |
It will only have a visible effect on manual downgrade from v3 to v2, and loaded into IPython 0.12, where everything will be perfectly functional, but all newlines will be duplicated. After this change, multiline entries in ipynb files with the multiline cell containing
to
|
OK I would probably just merge as is unless you really want to go through and update all the notebooks. But I don't think it is needed. |
OK with me too, reviewed in-person. |
Avoids discarding of information, e.g. `foo\rbar` should not be reconstructed as `foo\nbar`. Implemented in safe manner, where notebooks written by old scheme will load the same as before, and be updated on next save.
Keep line-endings in ipynb Avoids discarding of information, e.g. foo\rbar should not be reconstructed as foo\nbar. Implemented in safe manner, where notebooks written by old scheme will load the same as before, and be updated on next save.
Keep line-endings in ipynb Avoids discarding of information, e.g. foo\rbar should not be reconstructed as foo\nbar. Implemented in safe manner, where notebooks written by old scheme will load the same as before, and be updated on next save.
Avoids discarding of information, e.g.
foo\rbar
should not be reconstructed asfoo\nbar
.Implemented in safe manner, where notebooks written by old scheme will load the same as before, and be updated on next save.
Should address notebook loading issue brought up in #1659.