From 645ee13b0ec7707649fa2eb7c143d6f24e62271e Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 15 Sep 2021 15:39:28 -0400 Subject: [PATCH 1/5] Fixed crash when serializing dict with mix of string and int keys by not sorting keys (in either json engine) Add orjson OPT_NON_STR_KEYS and remove manual string conversion --- packages/python/plotly/plotly/io/_json.py | 6 +++--- .../plotly/plotly/tests/test_io/test_to_from_json.py | 3 +-- .../plotly/tests/test_io/test_to_from_plotly_json.py | 8 +++++++- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/python/plotly/plotly/io/_json.py b/packages/python/plotly/plotly/io/_json.py index 63c70101dd..c24fc6c910 100644 --- a/packages/python/plotly/plotly/io/_json.py +++ b/packages/python/plotly/plotly/io/_json.py @@ -112,7 +112,7 @@ def to_json_plotly(plotly_object, pretty=False, engine=None): # Dump to a JSON string and return # -------------------------------- if engine == "json": - opts = {"sort_keys": True} + opts = {} if pretty: opts["indent"] = 2 else: @@ -124,7 +124,7 @@ def to_json_plotly(plotly_object, pretty=False, engine=None): return json.dumps(plotly_object, cls=PlotlyJSONEncoder, **opts) elif engine == "orjson": JsonConfig.validate_orjson() - opts = orjson.OPT_SORT_KEYS | orjson.OPT_SERIALIZE_NUMPY + opts = orjson.OPT_NON_STR_KEYS | orjson.OPT_SERIALIZE_NUMPY if pretty: opts |= orjson.OPT_INDENT_2 @@ -462,7 +462,7 @@ def clean_to_json_compatible(obj, **kwargs): return obj if isinstance(obj, dict): - return {str(k): clean_to_json_compatible(v, **kwargs) for k, v in obj.items()} + return {k: clean_to_json_compatible(v, **kwargs) for k, v in obj.items()} elif isinstance(obj, (list, tuple)): if obj: # Must process list recursively even though it may be slow diff --git a/packages/python/plotly/plotly/tests/test_io/test_to_from_json.py b/packages/python/plotly/plotly/tests/test_io/test_to_from_json.py index 1c39203f14..a932282bd3 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_to_from_json.py +++ b/packages/python/plotly/plotly/tests/test_io/test_to_from_json.py @@ -29,9 +29,8 @@ def fig1(request): opts = { "separators": (",", ":"), "cls": plotly.utils.PlotlyJSONEncoder, - "sort_keys": True, } -pretty_opts = {"indent": 2, "cls": plotly.utils.PlotlyJSONEncoder, "sort_keys": True} +pretty_opts = {"indent": 2, "cls": plotly.utils.PlotlyJSONEncoder} # to_json diff --git a/packages/python/plotly/plotly/tests/test_io/test_to_from_plotly_json.py b/packages/python/plotly/plotly/tests/test_io/test_to_from_plotly_json.py index b97b76b822..e21b556c6b 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_to_from_plotly_json.py +++ b/packages/python/plotly/plotly/tests/test_io/test_to_from_plotly_json.py @@ -135,7 +135,7 @@ def datetime_array(request, datetime_value): def test_graph_object_input(engine, pretty): scatter = go.Scatter(x=[1, 2, 3], y=np.array([4, 5, 6])) result = pio.to_json_plotly(scatter, engine=engine) - expected = """{"type":"scatter","x":[1,2,3],"y":[4,5,6]}""" + expected = """{"x":[1,2,3],"y":[4,5,6],"type":"scatter"}""" assert result == expected check_roundtrip(result, engine=engine, pretty=pretty) @@ -215,3 +215,9 @@ def test_nonstring_key(engine, pretty): value = build_test_dict({0: 1}) result = pio.to_json_plotly(value, engine=engine) check_roundtrip(result, engine=engine, pretty=pretty) + + +def test_mixed_string_nonstring_key(engine, pretty): + value = build_test_dict({0: 1, "a": 2}) + result = pio.to_json_plotly(value, engine=engine) + check_roundtrip(result, engine=engine, pretty=pretty) From 69ddc6736a4751b65b23c70d918ac764e5a66a0c Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 15 Sep 2021 15:42:37 -0400 Subject: [PATCH 2/5] CHANGELOG --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30f70fd37d..5600ac8623 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## [next] - ??? + +### Fixed + - Fixed error when serializing dict with mix of string and non-string keys [#3380](https://github.com/plotly/plotly.py/issues/3380) + +### Updated + - The JSON serialization engines no longer sort their keys [#3380](https://github.com/plotly/plotly.py/issues/3380) + ## [5.3.1] - 2021-08-31 ### Updated From 21c7f70386a2836efbfd14faef4cd441ba37a885 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 15 Sep 2021 16:00:34 -0400 Subject: [PATCH 3/5] Test fix --- .../plotly/tests/test_core/test_graph_objs/test_template.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/python/plotly/plotly/tests/test_core/test_graph_objs/test_template.py b/packages/python/plotly/plotly/tests/test_core/test_graph_objs/test_template.py index a3061dfe5a..2cecffae58 100644 --- a/packages/python/plotly/plotly/tests/test_core/test_graph_objs/test_template.py +++ b/packages/python/plotly/plotly/tests/test_core/test_graph_objs/test_template.py @@ -356,7 +356,7 @@ def test_move_nested_trace_properties(self): }, ) - self.assertEqual(pio.to_json(templated_fig), pio.to_json(expected_fig)) + self.assertEqual(templated_fig.to_dict(), expected_fig.to_dict()) def test_move_nested_trace_properties_existing_traces(self): fig = go.Figure( From 46a87bd989128aa7d9c648379663872a216de630 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 15 Sep 2021 18:01:44 -0400 Subject: [PATCH 4/5] Remove layout string test --- .../plotly/tests/test_optional/test_offline/test_offline.py | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/python/plotly/plotly/tests/test_optional/test_offline/test_offline.py b/packages/python/plotly/plotly/tests/test_optional/test_offline/test_offline.py index dd9204947f..1f2a0bcc12 100644 --- a/packages/python/plotly/plotly/tests/test_optional/test_offline/test_offline.py +++ b/packages/python/plotly/plotly/tests/test_optional/test_offline/test_offline.py @@ -89,7 +89,6 @@ def test_default_mpl_plot_generates_expected_html(self): # just make sure a few of the parts are in here # like PlotlyOfflineTestCase(TestCase) in test_core self.assertTrue(data_json in html) # data is in there - self.assertTrue(layout_json in html) # layout is in there too self.assertTrue(PLOTLYJS in html) # and the source code # and it's an doc self.assertTrue(html.startswith("") and html.endswith("")) From c25e7105558f4c5bc8790273ec24d19b466cda1e Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 15 Sep 2021 18:36:33 -0400 Subject: [PATCH 5/5] Fix deepcopy/pickle tests --- .../plotly/tests/test_io/test_deepcopy_pickle.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/python/plotly/plotly/tests/test_io/test_deepcopy_pickle.py b/packages/python/plotly/plotly/tests/test_io/test_deepcopy_pickle.py index 4cbccdd2ba..245f6aaaa1 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_deepcopy_pickle.py +++ b/packages/python/plotly/plotly/tests/test_io/test_deepcopy_pickle.py @@ -38,7 +38,7 @@ def test_deepcopy_figure(fig1): fig_copied = copy.deepcopy(fig1) # Contents should be equal - assert pio.to_json(fig_copied) == pio.to_json(fig1) + assert fig_copied.to_dict() == fig1.to_dict() # Identities should be distinct assert fig_copied is not fig1 @@ -50,7 +50,7 @@ def test_deepcopy_figure_subplots(fig_subplots): fig_copied = copy.deepcopy(fig_subplots) # Contents should be equal - assert pio.to_json(fig_copied) == pio.to_json(fig_subplots) + assert fig_copied.to_dict() == fig_subplots.to_dict() # Subplot metadata should be equal assert fig_subplots._grid_ref == fig_copied._grid_ref @@ -66,7 +66,7 @@ def test_deepcopy_figure_subplots(fig_subplots): fig_copied.add_bar(y=[0, 0, 1], row=1, col=2) # And contents should be still equal - assert pio.to_json(fig_copied) == pio.to_json(fig_subplots) + assert fig_copied.to_dict() == fig_subplots.to_dict() def test_deepcopy_layout(fig1): @@ -91,21 +91,21 @@ def test_pickle_figure_round_trip(fig1): fig_copied = pickle.loads(pickle.dumps(fig1)) # Contents should be equal - assert pio.to_json(fig_copied) == pio.to_json(fig1) + assert fig_copied.to_dict() == fig1.to_dict() def test_pickle_figure_subplots_round_trip(fig_subplots): fig_copied = pickle.loads(pickle.dumps(fig_subplots)) # Contents should be equal - assert pio.to_json(fig_copied) == pio.to_json(fig_subplots) + assert fig_copied.to_dict() == fig_subplots.to_dict() # Should be possible to add new trace to subplot location fig_subplots.add_bar(y=[0, 0, 1], row=1, col=2) fig_copied.add_bar(y=[0, 0, 1], row=1, col=2) # And contents should be still equal - assert pio.to_json(fig_copied) == pio.to_json(fig_subplots) + assert fig_copied.to_dict() == fig_subplots.to_dict() def test_pickle_layout(fig1):