From b1cd7d1682fe85918dc80295f001235813ecec16 2014-12-15 18:28:51 From: Min RK Date: 2014-12-15 18:28:51 Subject: [PATCH] JSON formatter expects JSONable dict/list not already-serialized JSON string pre-3.0 case of JSON string is handled with a warning. msgspec5 made this change, but Python APIs that publish JSON weren't updated to match. --- diff --git a/IPython/core/display.py b/IPython/core/display.py index 0915ede..0d07fdf 100644 --- a/IPython/core/display.py +++ b/IPython/core/display.py @@ -1,27 +1,16 @@ # -*- coding: utf-8 -*- -"""Top-level display functions for displaying object in different formats. +"""Top-level display functions for displaying object in different formats.""" -Authors: - -* Brian Granger -""" - -#----------------------------------------------------------------------------- -# Copyright (C) 2013 The IPython Development Team -# -# Distributed under the terms of the BSD License. The full license is in -# the file COPYING, distributed as part of this software. -#----------------------------------------------------------------------------- - -#----------------------------------------------------------------------------- -# Imports -#----------------------------------------------------------------------------- +# Copyright (c) IPython Development Team. +# Distributed under the terms of the Modified BSD License. from __future__ import print_function +import json +import mimetypes import os import struct -import mimetypes +import warnings from IPython.core.formatters import _safe_get_formatter_method from IPython.utils.py3compat import (string_types, cast_bytes_py2, cast_unicode, @@ -514,7 +503,29 @@ class SVG(DisplayObject): return self.data -class JSON(TextDisplayObject): +class JSON(DisplayObject): + """JSON expects a JSON-able dict or list + + not an already-serialized JSON string. + + Scalar types (None, number, string) are not allowed, only dict or list containers. + """ + # wrap data in a property, which warns about passing already-serialized JSON + _data = None + def _check_data(self): + if self.data is not None and not isinstance(self.data, (dict, list)): + raise TypeError("%s expects JSONable dict or list, not %r" % (self.__class__.__name__, self.data)) + + @property + def data(self): + return self._data + + @data.setter + def data(self, data): + if isinstance(data, string_types): + warnings.warn("JSON expects JSONable dict or list, not JSON strings") + data = json.loads(data) + self._data = data def _repr_json_(self): return self.data diff --git a/IPython/core/formatters.py b/IPython/core/formatters.py index 710a221..d7b6f81 100644 --- a/IPython/core/formatters.py +++ b/IPython/core/formatters.py @@ -12,6 +12,7 @@ Inheritance diagram: import abc import inspect +import json import sys import traceback import warnings @@ -232,15 +233,7 @@ def warn_format_error(method, self, *args, **kwargs): else: traceback.print_exception(*exc_info) return None - if r is None or isinstance(r, self._return_type) or \ - (isinstance(r, tuple) and r and isinstance(r[0], self._return_type)): - return r - else: - warnings.warn( - "%s formatter returned invalid type %s (expected %s) for object: %s" % \ - (self.format_type, type(r), self._return_type, _safe_repr(args[0])), - FormatterWarning - ) + return self._check_return(r, args[0]) class FormatterABC(with_metaclass(abc.ABCMeta, object)): @@ -259,7 +252,6 @@ class FormatterABC(with_metaclass(abc.ABCMeta, object)): enabled = True @abc.abstractmethod - @warn_format_error def __call__(self, obj): """Return a JSON'able representation of the object. @@ -358,6 +350,21 @@ class BaseFormatter(Configurable): else: return True + def _check_return(self, r, obj): + """Check that a return value is appropriate + + Return the value if so, None otherwise, warning if invalid. + """ + if r is None or isinstance(r, self._return_type) or \ + (isinstance(r, tuple) and r and isinstance(r[0], self._return_type)): + return r + else: + warnings.warn( + "%s formatter returned invalid type %s (expected %s) for object: %s" % \ + (self.format_type, type(r), self._return_type, _safe_repr(obj)), + FormatterWarning + ) + def lookup(self, obj): """Look up the formatter for a given instance. @@ -794,16 +801,41 @@ class LatexFormatter(BaseFormatter): class JSONFormatter(BaseFormatter): """A JSON string formatter. - To define the callables that compute the JSON string representation of + To define the callables that compute the JSONable representation of your objects, define a :meth:`_repr_json_` method or use the :meth:`for_type` or :meth:`for_type_by_name` methods to register functions that handle this. - The return value of this formatter should be a valid JSON string. + The return value of this formatter should be a JSONable list or dict. + JSON scalars (None, number, string) are not allowed, only dict or list containers. """ format_type = Unicode('application/json') + _return_type = (list, dict) print_method = ObjectName('_repr_json_') + + def _check_return(self, r, obj): + """Check that a return value is appropriate + + Return the value if so, None otherwise, warning if invalid. + """ + if r is None: + return + md = None + if isinstance(r, tuple): + # unpack data, metadata tuple for type checking on first element + r, md = r + + # handle deprecated JSON-as-string form from IPython < 3 + if isinstance(r, string_types): + warnings.warn("JSON expects JSONable list/dict containers, not JSON strings", + FormatterWarning) + r = json.loads(r) + + if md is not None: + # put the tuple back together + r = (r, md) + return super(JSONFormatter, self)._check_return(r, obj) class JavascriptFormatter(BaseFormatter): diff --git a/IPython/core/magics/basic.py b/IPython/core/magics/basic.py index c4c48cb..11da19b 100644 --- a/IPython/core/magics/basic.py +++ b/IPython/core/magics/basic.py @@ -64,7 +64,7 @@ class MagicsDisplay(object): return magic_dict def _repr_json_(self): - return json.dumps(self._jsonable()) + return self._jsonable() @magics_class diff --git a/IPython/core/tests/test_display.py b/IPython/core/tests/test_display.py index 2e8a5d1..177ec72 100644 --- a/IPython/core/tests/test_display.py +++ b/IPython/core/tests/test_display.py @@ -1,11 +1,9 @@ -#----------------------------------------------------------------------------- -# Copyright (C) 2010-2011 The IPython Development Team. -# -# Distributed under the terms of the BSD License. -# -# The full license is in the file COPYING.txt, distributed with this software. -#----------------------------------------------------------------------------- +# Copyright (c) IPython Development Team. +# Distributed under the terms of the Modified BSD License. + +import json import os +import warnings import nose.tools as nt @@ -130,3 +128,21 @@ def test_displayobject_repr(): nt.assert_equal(repr(j), object.__repr__(j)) j._show_mem_addr = False nt.assert_equal(repr(j), '') + +def test_json(): + d = {'a': 5} + lis = [d] + j = display.JSON(d) + nt.assert_equal(j._repr_json_(), d) + with warnings.catch_warnings(record=True) as w: + j = display.JSON(json.dumps(d)) + assert len(w) == 1 + nt.assert_equal(j._repr_json_(), d) + j = display.JSON(lis) + nt.assert_equal(j._repr_json_(), lis) + with warnings.catch_warnings(record=True) as w: + j = display.JSON(json.dumps(lis)) + assert len(w) == 1 + nt.assert_equal(j._repr_json_(), lis) + + \ No newline at end of file diff --git a/IPython/core/tests/test_formatters.py b/IPython/core/tests/test_formatters.py index 7956efc..b072f0e 100644 --- a/IPython/core/tests/test_formatters.py +++ b/IPython/core/tests/test_formatters.py @@ -1,5 +1,6 @@ """Tests for the Formatters.""" +import warnings from math import pi try: @@ -8,10 +9,11 @@ except: numpy = None import nose.tools as nt +from IPython import get_ipython from IPython.config import Config from IPython.core.formatters import ( PlainTextFormatter, HTMLFormatter, PDFFormatter, _mod_name_key, - DisplayFormatter, + DisplayFormatter, JSONFormatter, ) from IPython.utils.io import capture_output @@ -418,3 +420,14 @@ def test_ipython_display_formatter(): nt.assert_equal(md, {}) nt.assert_equal(catcher, [yes]) +def test_json_as_string_deprecated(): + class JSONString(object): + def _repr_json_(self): + return '{}' + + f = JSONFormatter() + with warnings.catch_warnings(record=True) as w: + d = f(JSONString()) + nt.assert_equal(d, {}) + nt.assert_equal(len(w), 1) + \ No newline at end of file diff --git a/IPython/core/tests/test_magic.py b/IPython/core/tests/test_magic.py index a8b9cc6..d4e5104 100644 --- a/IPython/core/tests/test_magic.py +++ b/IPython/core/tests/test_magic.py @@ -8,6 +8,7 @@ from __future__ import absolute_import import io import os import sys +import warnings from unittest import TestCase, skipIf try: @@ -18,6 +19,7 @@ except ImportError: import nose.tools as nt +from IPython import get_ipython from IPython.core import magic from IPython.core.error import UsageError from IPython.core.magic import (Magics, magics_class, line_magic, @@ -980,3 +982,13 @@ def test_bookmark(): with tt.AssertPrints('bmname'): ip.run_line_magic('bookmark', '-l') ip.run_line_magic('bookmark', '-d bmname') + +def test_ls_magic(): + ip = get_ipython() + json_formatter = ip.display_formatter.formatters['application/json'] + json_formatter.enabled = True + lsmagic = ip.magic('lsmagic') + with warnings.catch_warnings(record=True) as w: + j = json_formatter(lsmagic) + nt.assert_equal(sorted(j), ['cell', 'line']) + nt.assert_equal(w, []) # no warnings