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

notepad + jsonlib: TypeError: Only whitespace may be used for indentation. #1037

Closed
wolever opened this issue Nov 24, 2011 · 8 comments
Closed
Milestone

Comments

@wolever
Copy link
Contributor

wolever commented Nov 24, 2011

When jsonlib is installed, the notepad crashes with a TypeError:

Traceback (most recent call last):
...
  File "/Users/wolever/code/sandbox/env/sandbox/lib/python2.6/site-packages/IPython/zmq/kernelmanager.py", line 752, in start_kernel
    self.write_connection_file()
  File "/Users/wolever/code/sandbox/env/sandbox/lib/python2.6/site-packages/IPython/zmq/kernelmanager.py", line 716, in write_connection_file
    shell_port=self.shell_port, hb_port=self.hb_port)
  File "/Users/wolever/code/sandbox/env/sandbox/lib/python2.6/site-packages/IPython/zmq/entry_point.py", line 86, in write_connection_file
    f.write(json.dumps(cfg, indent=2))
  File "/Users/wolever/code/sandbox/env/sandbox/lib/python2.6/site-packages/zmq/utils/jsonapi.py", line 67, in jsonlib_dumps
    return _squash_unicode(jsonmod.dumps(o,**kwargs))
  File "/Users/wolever/code/sandbox/env/sandbox/lib/python2.6/site-packages/jsonlib.py", line 799, in write
    return write_impl (value, sort_keys, validate_indent (indent), ascii_only,
  File "/Users/wolever/code/sandbox/env/sandbox/lib/python2.6/site-packages/jsonlib.py", line 812, in validate_indent
    raise TypeError ("Only whitespace may be used for indentation.")
TypeError: Only whitespace may be used for indentation.

It looks like this is because jsonlib expects the indent argument to be string, while the stdlib's json expects indent to be a number. The offending code is below:

> /Users/wolever/code/sandbox/env/sandbox/lib/python2.6/site-packages/IPython/zmq/entry_point.py(86)write_connection_file()
-> f.write(json.dumps(cfg, indent=2))
(Pdb) list
 81                   )
 82         cfg['ip'] = ip
 83         cfg['key'] = bytes_to_str(key)
 84         
 85         with open(fname, 'wb') as f:
 86  ->         f.write(json.dumps(cfg, indent=2))
 87         
 88         return fname, cfg
 89         
 90     
 91     def base_launch_kernel(code, fname, stdin=None, stdout=None, stderr=None,

The above is Python 2.6.7 on OS X 10.7 with jsonlib 1.6.1.

Uninstalling jsonlib (presumably forcing a fallback to stdlib json?) fixes this issue.

@fperez
Copy link
Member

fperez commented Nov 24, 2011

Yup, good catch. @minrk, we should probably work around this one in ipython regardless, I'll make the fixes now so that we support jsonlib as well. But ultimately it seems to me the deeper fix should be made in zmq.jsonapi, right?

@fperez fperez closed this as completed in 8cacf9c Nov 24, 2011
@fperez
Copy link
Member

fperez commented Nov 24, 2011

OK, I've fixed it quickly, but I noticed that with jsonlib installed, the full test suite doesn't quite complete.

@minrk, I had to manually interrupt it at:

test_retries (IPython.parallel.tests.test_lbview.TestLoadBalancedView) ... ^CTraceback (most recent call last):
  File "/home/fperez/usr/lib/python2.7/site-packages/IPython/testing/iptest.py", line 313, in run
    return self._run_cmd()
  File "/home/fperez/usr/lib/python2.7/site-packages/IPython/testing/iptest.py", line 306, in _run_cmd
    retcode = subp.wait()
  File "/usr/lib/python2.7/subprocess.py", line 1281, in wait
    pid, sts = _eintr_retry_call(os.waitpid, self.pid, 0)
  File "/usr/lib/python2.7/subprocess.py", line 478, in _eintr_retry_call
    return func(*args)
KeyboardInterrupt

I don't know if you want to consider that an issue for us, for zmq or for jsonlib, though, so I won't open another one quite yet. If you think it's on our side, then let's open a new issue for it.

@minrk
Copy link
Member

minrk commented Nov 24, 2011

The goal of jsonlib was to make fast json (stdlib json is extremely slow). but they decided to set their own API. Then, jsonlib2 project was started to bring jsonlib back to API compatibility.

I might just drop jsonlib support from zmq, but that's irrelevant to IPython, unless we bump the minimum version to 2.1.11.

In the notebook, we can just use stdlib json, because performance is irrelevant. I'll look into the parallel issue, because I can't think of a reason why it shouldn't work.

@fperez
Copy link
Member

fperez commented Nov 24, 2011

Ok, thanks. In the meantime, the workaround I committed means people won't get burned too badly if they have jsonlib installed; I'll leave the decision of what to do in zmq up to you (though dropping jsonlib support seems like the right approach: I looked and the api is so different that any but the most trivial uses are likely to cause problems similar to this one).

@minrk
Copy link
Member

minrk commented Nov 24, 2011

On Nov 24, 2011, at 13:52, Fernando Perezreply@reply.github.com wrote:

Ok, thanks. In the meantime, the workaround I committed means people won't get burned too badly if they have jsonlib installed; I'll leave the decision of what to do in zmq up to you (though dropping jsonlib support seems like the right approach: I looked and the api is so different that any but the most trivial uses are likely to cause problems similar to this one).

Yes, well presumably most users who have chosen to install jsonlib would know how to use it. It's just projects like IPython that have an extra layer of abstraction on zmq, that have to take this kind of thing into account, and even then, only if they want to customize json output (it's irrelevant if extra kwargs aren't being specified).


Reply to this email directly or view it on GitHub:
#1037 (comment)

@fperez
Copy link
Member

fperez commented Nov 24, 2011

On Thu, Nov 24, 2011 at 1:57 PM, Min RK
reply@reply.github.com
wrote:

Yes, well presumably most users who have chosen to install jsonlib would know how to use it.

True, though sometimes stuff gets pulled in as a dependency, which can
cause this kind of problem: project A uses zmq.jsonapi in a way that's
incompatible with jsonlib but nobody has noticed, then user installs
project B that requires jsonlib, and then all of a sudden project A
breaks on their machine. So the current situation is a little
brittle.

@minrk
Copy link
Member

minrk commented Nov 25, 2011

The main point of the zmq.utils.jsonapi is that it serializes to/from utf8-encoded bytes, and json libraries are inconsistent in whether they deal in unicode or str. If that's not what we need (e.g. talking to files), we should be using the stdlib json. And the fact that jsonlib does not guarantee kwarg equivalence means that anyone using jsonapi with kwargs must take this into account (jsonapi.jsonmod.__name__ is the appropriate value to check).

The practical result of this is that we (IPython) should essentially never pass kwargs when using zmq.jsonapi. There is exactly one case where we are currently using a kwarg where we should be using zmq.jsonapi at all, and it has always properly handled the difference in jsonlib. The real bug here is that we were using jsonapi in places that don't make any sense.

I did find the bug with jsonlib in the test suite - it unserializes floats to decimal.Decimal (?!), so I have to cast it to float before passing it to datetime.timedelta. There are few cases where the difference will matter (and exactly one case where a float is ever sent via json), but we will have to be wary if we work with floats while jsonlib is a possible actor.

This may be the last straw for dropping jsonlib support in pyzmq, but that will be irrelevant to IPython until we bump minimum pyzmq version past 2.1.11.

A monkeypatch for prohibiting pyzmq from using jsonlib, and forcing fallback to stdlib:

import json
from zmq.utils import jsonapi

if jsonapi.jsonmod.__name__ == 'jsonlib':
    jsonapi.jsonmod =  json

But this may not be ideal, as jsonlib is significantly faster than stdlib, and with the small fix in PR #1040, well behaved.

@fperez
Copy link
Member

fperez commented Nov 25, 2011

OK, I like the approach in #1040, so let's go with that. Thanks!

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
Note that there seems to be a deeper problem with jsonlib; at least on
my system it doesn't pass the full test suite.  But with this fix,
regular interactive use is now OK.

Closes ipython#1037.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants