Py3 tests2 #913

Merged
merged 5 commits into from Oct 28, 2011

Conversation

Projects
None yet
3 participants
Owner

takluyver commented Oct 20, 2011

Fixes for a couple of tests under Python 3.

@ellisonbg, I'm having some trouble with a test in nbformat. With these changes, I get:

======================================================================
FAIL: test_roundtrip (IPython.nbformat.v2.tests.test_json.TestJSON)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/thomas/Code/virtualenvs/ipy-2to3/lib/python3.2/site-packages/ipython-0.12.dev-py3.2.egg/IPython/nbformat/v2/tests/test_json.py", line 18, in test_roundtrip
    self.assertEquals(reads(s),nb0)

followed by a long dump of the before and after dicts - I can't even spot the != in there, but I think the difference is between:

{'latex': '$a$', 'prompt_number': 3, 'svg': '<svg>', 'javascript': 'var i=0;', 'jpeg': 'ZGF0YQ==\n', 'json': 'json data', 'html': 'The HTML rep', 'text': '<array a>', 'output_type': 'pyout', 'png': 'ZGF0YQ==\n'}

and (N.B. b'data'):

{'latex': '$a$', 'prompt_number': 3, 'svg': '<svg>', 'javascript': 'var i=0;', 'jpeg': b'data', 'json': 'json data', 'html': 'The HTML rep', 'text': '<array a>', 'output_type': 'pyout', 'png': b'data'}

Any immediate thoughts?

Owner

minrk commented Oct 22, 2011

There are definitely some bugs in the string/unicode/bytes logic in nbformat. In the JSON roundtrip, the problem is that the inverse of the b64 encode is never called. This doesn't come up, because the bytes are already b64-encoded in actual practice, and the bytes-logic in the JSONEncoder is actually never called in Python2 (as far as I can tell).

I think the real bug is that the base64_encode/decode functions in rwbase encode cell.png / cell.jpeg, but as far as I can tell, those attributes never exist. png/jpegs are attached to outputs of a cell, not the cells themselves.

This doesn't come up in Python2, because that output appears to always already be b64-encoded bytes, which pass right through to/from JSON without issue.

Owner

takluyver commented Oct 22, 2011

Is there anything simple that can be done to fix it? I'm not at all familiar with this part of the code.

Owner

minrk commented Oct 22, 2011

Further detail: every bytes object in the notebook is already a b64-encoded bytes object. So, instead of performing b64 encoding/decoding around JSON, it should be an ascii encoding/decoding, leaving the contents unchanged.

Owner

minrk commented Oct 22, 2011

Yes, I've got it working locally - the answer is to stop using b64 to/from JSON, as it has already been encoded. I'll PR to your branch shortly with fixes after I clean up all my code.

@minrk minrk fix base64 code in nbformat.v2
base64 encoding functions were called, but had no effect, because
the notebook already has everything as b64-encoded bytestrings, which
are valid ascii literals on Python 2.

However, the encode/decode logic is actually triggered on Python 3, revealing its errors.

This fixes the base64 functions that had no effect to have their intended effect,
but does not use them.  Rather, it is assumed that
bytes objects are already b64-encoded (and thus ascii-safe), which
assumption was already made in Python 2.
155b20c
Owner

minrk commented Oct 22, 2011

PR issued

Owner

takluyver commented Oct 22, 2011

Thanks Min - I've added that to this PR, and the nbformat tests are passing now.

@minrk minrk and 1 other commented on an outdated diff Oct 23, 2011

IPython/core/tests/test_magic.py
@@ -324,7 +324,10 @@ def check_cpaste(code, should_fail=False):
_ip.user_ns['code_ran'] = False
src = StringIO()
- src.encoding = None # IPython expects stdin to have an encoding attribute
+ try:
+ src.encoding = None # IPython expects stdin to have an encoding attribute
+ except Exception:
+ pass # ...but it's a read-only attribute in Python 3
@minrk

minrk Oct 23, 2011

Owner

Shouldn't this be 'if not hasattr', rather than try/except? We don't need to do it if it already exists.

@takluyver

takluyver Oct 26, 2011

Owner

Good point. Done.

Owner

takluyver commented Oct 26, 2011

I've also made a couple of decorators to mark known failures, so it's easier to spot regressions. The parallel package is still failing - I'll try to look into it if I get time, but I think it's fairly low priority.

Owner

fperez commented Oct 28, 2011

This is looking pretty solid, so I'll merge it now to avoid bitrot. When you get a chance to tackle the parallel ones, a new PR can come in for those. Thanks for this work! Looking better and better on py3 :)

@fperez fperez added a commit that referenced this pull request Oct 28, 2011

@fperez fperez Merge pull request #913 from takluyver/py3-tests2
Fix more tests under Python3.  The test suite isn't fully fixed yet under py3, but this gets us closer.
9463121

@fperez fperez merged commit 9463121 into ipython:master Oct 28, 2011

@mattvonrocketstein mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

@fperez fperez Merge pull request #913 from takluyver/py3-tests2
Fix more tests under Python3.  The test suite isn't fully fixed yet under py3, but this gets us closer.
767a0cb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment