rename plaintext cell -> raw cell #1490
Conversation
|
I prefer "rawtext" to "raw" as it is a bit more explicit. Don't know what to think about existing v3 notebooks. By now, there are a lot of them around. I think people will be upset if those stop working. We could increment to v4 or we could add extra code to handle the other name on load. |
|
I think we definitely shouldn't increment to v4. Incrementing the format is immensely painful, and the only reason we merged the new format before was because we decided it was acceptable to change the notebook format in master, not because it was considered final. We can add some simple checks to allow the 'plaintext' name on input, which will silently be renamed to 'rawtext' in memory / on next save. |
|
Also, on the name - as always, redundant code such as |
|
On Wed, Mar 14, 2012 at 12:33 PM, Min RK
I agree that is a bit redundant - I am less worried about the code Cheers, Brian
Brian E. Granger |
|
On Wed, Mar 14, 2012 at 12:29 PM, Min RK
Yep, incrementing the format is very painful. As long as the code to
Brian E. Granger |
|
Okay, rename is now 'plaintext' -> 'rawtext', and old plaintext cells open just fine, and are renamed to rawtext immediately on load (in nb readers / Notebook.fromJSON ). |
|
i wanted to chime in with not incrementing nbformat and just changing it at will in master. It makes sense to only increment version numbers AFTER a particular format has made it into an official release |
|
Min, sorry if I wasn't clear before. I think your usage of the name "raw" in the non-user facing code is fine (the nbformat, internal Javascript code, etc.). I even agree with you that it is redundant to have a text cell that is "rawtext". The only place where I think it makes sense to call it "rawtext" is in the UI elements (menu's, keyboard shortcuts, etc.). In that context, the word "raw" alone doesn't indicate that it is a text cell. But there are only a few places that users see those words. Does this make sense? |
|
On reading the code, it's really only the Python readers/writers for which it is redundant, and most places where you see cell_type there isn't immediate info that it's also text, so I agree that 'rawtext' is better. |
|
@ivanov we try to keep the version increments to a minimum, but anytime we make changes to the actual notebook format in master, we have to increment the version, auto update older notebooks etc. Otherwise users will see confusing and broken behavior. IOW, the issue is not just with the nbformat # but with the entire format. If it changes, we run into these issues. What is difficult is that users are tracking master so closely and using it for basically production usage (myself included). They want the features of master, but the stability of a stable release nbformat. Almost the second we make a change, people are using it, sharing notebooks based on it, etc. I don't know what the best way of handling these things is. Overall, I am very much -1 on the type of thing we are doing here to avoid incrementing the nbformat because we have to carry those special cases forward for a long time. The best answer is to probably batch changes to the nbformat and make all of them at once and increment the nbformat # at that time. If we went with that approach, the current PR would just have to wait until the next batch of nbformat related work and the v4 increment. The alternative is to tell people "if you use the dev version of the notebook, you may end up with notebooks that are incompatible with released versions of the notebook. That doesn't sounds very nice though. @fperez do you have any thoughts on this important issue? |
|
OK sounds good. On Wed, Mar 14, 2012 at 1:12 PM, Min RK
Brian E. Granger |
|
I'm actually quite torn, b/c of the flow of things you describe correctly. But after seeing the amount of usage in the wild of the notebook, I think we really need to become as paranoid as possible with breaking anything on the user side. Perhaps we'll need to do batching of PRs that affect the format as you suggest, in any case where carrying compatibility codes is hackish. One thing we should do, is to put all such special-casing code in a single location, so that it can be found easily and removed when no longer needed. Otherwise those things will linger forever. |
|
Okay, per Brian's comments cell_type for raw cells is once again 'raw', but UI says 'Raw Text'. I still have the full 'rawtext' change stashed if we want that instead. commit d2b70d8 is the entire change needed to support opening notebooks that use the deprecated 'plaintext' identifier. one-two lines in three places (NB reader base, .py reader, javascript reader). For that reason, as well as the very-low usage of this cell type at this point (we have zero examples in the docs), I think this change is acceptable before release. I think it's quite important that we change this name before releasing anything called 'plaintext', because the logical interpretation of 'plaintext' for a notebook reader and/or writer is not what we mean these cells to be. How I would interpret plaintext: This text should be preserved after rendering the intermediate format (e.g. And Raw: preserved as untransformed input to the export format (e.g. raw RST, or raw LaTeX). If others don't have this perception, then I wouldn't worry about it, and just close this PR. |
|
I agree that 'raw' is a better description of our intent, and I'm +1 on using 'raw' in the code and 'Raw Text' in the UI. The changes are indeed pretty small, though I don't know when we'd be OK removing them in the future. At least we should have a convention to mark such code, so that we can grep for it in the future. A special comment such as |
|
I agree with your being +1 on the current PR. I think the only issue On Wed, Mar 14, 2012 at 5:22 PM, Fernando Perez
Brian E. Granger |
|
So guys, what do we do with this one? It looks like we're OK merging it in its current state, even if it means carrying around a bit of special-casing version-dependent code? After the last month's events, our notebook user base has probably gone up by a huge amount, which means that breaking things for users is even a worse idea now than when we last discussed it here, about a month ago... But we should really make a decision on this one so we can close our PR backlog... |
|
I didn't add the comments marking the 'plaintext' handling, but I can do that now. I think the only people this should actually affect are those who were working on the nbformat translators, who should update to use the new key. |
|
VERSIONHACK comments added |
|
Ok, this is going in... |
rename plaintext cell -> raw cell Raw cells should be *untransformed* when writing various output formats, as the point of them is to let users pass through IPython to their rendered document format (rst, latex, etc.). This is different from what is the logical meaning of 'plaintext', which would suggest that the contents should be preserved as unformatted plaintext (e.g. in a `<pre>` tag, or literal block). In the UI, these cells will be displayed as 'Raw Text'. WARNING: any existing v3 notebooks which use plaintext cells, when read in by versions after this merge, will silently rename those cells to 'raw'. But if such a notebook is uploaded into a pre-merge IPython, cells labeled as 'raw' will simply *not be displayed*.
rename plaintext cell -> raw cell Raw cells should be *untransformed* when writing various output formats, as the point of them is to let users pass through IPython to their rendered document format (rst, latex, etc.). This is different from what is the logical meaning of 'plaintext', which would suggest that the contents should be preserved as unformatted plaintext (e.g. in a `<pre>` tag, or literal block). In the UI, these cells will be displayed as 'Raw Text'. WARNING: any existing v3 notebooks which use plaintext cells, when read in by versions after this merge, will silently rename those cells to 'raw'. But if such a notebook is uploaded into a pre-merge IPython, cells labeled as 'raw' will simply *not be displayed*.
'raw' makes more sense to me as a name for these cells, regarding what we actually intend them to mean.
Raw cells should be untransformed when writing various output formats, as the point of them is to let users pass through IPython to their rendered document format (rst, latex, etc.). This is different from what is the logical meaning of 'plaintext', which would suggest that the contents should be preserved as unformatted plaintext (e.g. in a
<pre>tag, or literal block).Before merging, we should decide what we should do about existing v3 notebooks which use plaintext cells. As this PR currently stands, these cells will simply be unrecognized.