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 #1581 #1582

Merged
merged 3 commits into from Jan 17, 2015
Merged

Fix #1581 #1582

merged 3 commits into from Jan 17, 2015

Conversation

@mforbes
Copy link
Contributor

@mforbes mforbes commented Jan 17, 2015

This adds a bit of code to deal with the API changes in the latest development version of IPython 3.0.0. Note: I read the notebook into the current format (as opposed to nbformat.NO_CONVERT) so that the HTMLExporter can properly output the data. If the conversion is not done, then this will fail if there are cells in the notebook as shown in the issue.

There is probably more that could be cleaned up (the generated notebook for an initial post should probably use the current format rather than a hard-coded nbformat 3 template for example), but I wanted to keep this a minimal change to preserve backward compatibility.

return IPython.nbformat.reads(
s, IPython.nbformat.current_nbformat)
else:
nbformat_reads = IPython.nbformat.current.reads_json
Copy link
Member

@Kwpolska Kwpolska Jan 17, 2015

Can’t you just use plain reads on all versions?

nb_json = IPython.nbformat.reads(s)

seems to be enough on v2 and v3, at least according to the docs.

Copy link
Member

@Kwpolska Kwpolska Jan 17, 2015

Also, you would have to import IPython.nbformat.current and work with it all the time.

Here is a revised solution:

Copy link
Member

@Kwpolska Kwpolska Jan 17, 2015

if IPython.version_info[0] >= 3:
    nbformat_reads = lambda s: IPython.nbformat.reads(s)
else:
    import IPython.nbformat.current
    nbformat_reads = IPython.nbformat.current.reads_json

Copy link
Contributor Author

@mforbes mforbes Jan 17, 2015

I believe that reads() requires a format as a second argument.

Copy link
Member

@Kwpolska Kwpolska Jan 17, 2015

Ah yes, it does. I was reading the wrong file.

    nbformat_reads = lambda s: IPython.nbformat.reads(s, IPython.nbformat.current_nbformat)

@mforbes
Copy link
Contributor Author

@mforbes mforbes commented Jan 17, 2015

I think this should work but have not tested it with IPython 2.0 yet...

current_nbformat = nbformat.current_nbformat
else:
import IPython.nbformat.current as nbformat
current_nbformat = 'json'
Copy link
Contributor Author

@mforbes mforbes Jan 17, 2015

Maybe this should be a Unicode string (u'json')? (I don't think it matters though, but have not really done much with unicode strings yet...)

Copy link
Member

@Kwpolska Kwpolska Jan 17, 2015

It already is Unicode:

from __future__ import unicode_literals, print_function
                       ^^^^^^^^^^^^^^^^

Copy link

@muxuezi muxuezi Mar 4, 2015

cool! It's work after I upgrade to IPython3.0.
Thanks!

@Kwpolska Kwpolska added this to the v7.3.1 milestone Jan 17, 2015
@Kwpolska Kwpolska self-assigned this Jan 17, 2015
@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Jan 17, 2015

Works fine with v2.3.1:

In [1]: import IPython.nbformat.current as nbformat

In [2]: import io

In [3]: in_file = io.open('plot-test.ipynb', 'r', encoding='utf-8')

In [4]: nb_json = nbformat.read(in_file, u'json')

In [5]: nb_json
Out[5]: 
{'nbformat': 3,
 'nbformat_minor': 0,
…

In [6]: type(_)
Out[6]: IPython.nbformat.v3.nbbase.NotebookNode

If it works with the current v3 dev:

  • add yourself to /AUTHORS.txt

  • add this to /CHANGES.txt (on the top of the file):

    New in master
    =============
    
    Features
    --------
    
    Bugfixes
    --------
    
    * Fixed compatibility with IPython 3.x (Issue #1582)

Merging as soon as this is done.

@mforbes
Copy link
Contributor Author

@mforbes mforbes commented Jan 17, 2015

Shouldn't that be: Issue #1581?

* Fixed compatibility with IPython 3.x (Issue #1581)

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Jan 17, 2015

Sure.

Kwpolska added a commit that referenced this issue Jan 17, 2015
@Kwpolska Kwpolska merged commit ddaa0dc into getnikola:master Jan 17, 2015
0 of 2 checks passed
@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Jan 17, 2015

👍 Thanks for your contribution!

@mforbes mforbes deleted the fix-1581 branch Jan 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants