From 0dc50d9fe5ff2c8c0289d916ae5d3ae01bccdd97 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 19 Jun 2019 08:09:32 -0400 Subject: [PATCH 01/11] Have side-effect renderers show figure on display mimetype renderers already did this. Now, px can be used easily with the 'browser' renderer for non-jupyter contexts. --- packages/python/plotly/plotly/basedatatypes.py | 16 +++++----------- packages/python/plotly/plotly/basewidget.py | 9 +++++++++ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/packages/python/plotly/plotly/basedatatypes.py b/packages/python/plotly/plotly/basedatatypes.py index 40c5437d31f..ba55581904e 100644 --- a/packages/python/plotly/plotly/basedatatypes.py +++ b/packages/python/plotly/plotly/basedatatypes.py @@ -427,22 +427,16 @@ def __repr__(self): return repr_str - def _repr_mimebundle_(self, include, exclude, **kwargs): + def _ipython_display_(self): """ - repr_mimebundle should accept include, exclude and **kwargs + Handle rich display of figures in ipython contexts """ import plotly.io as pio - if pio.renderers.render_on_display: - data = pio.renderers._build_mime_bundle(self.to_dict()) - - if include: - data = {k: v for (k, v) in data.items() if k in include} - if exclude: - data = {k: v for (k, v) in data.items() if k not in exclude} - return data + if pio.renderers.render_on_display and pio.renderers.default: + pio.show(self) else: - return None + print (repr(self)) def update(self, dict1=None, **kwargs): """ diff --git a/packages/python/plotly/plotly/basewidget.py b/packages/python/plotly/plotly/basewidget.py index 398ffd2ed14..02e7ed2f9f5 100644 --- a/packages/python/plotly/plotly/basewidget.py +++ b/packages/python/plotly/plotly/basewidget.py @@ -730,6 +730,15 @@ def _handler_js2py_pointsCallback(self, change): self._js2py_pointsCallback = None + # Display + # ------- + def _ipython_display_(self): + """ + Handle rich display of figures in ipython contexts + """ + # Override BaseFigure's display to make sure we display the widget version + widgets.DOMWidget._ipython_display_(self) + # Callbacks # --------- def on_edits_completed(self, fn): From ff0e069cabc0917f6f734fa3eca07f388f197812 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 19 Jun 2019 09:21:33 -0400 Subject: [PATCH 02/11] Fix renderer tests --- .../tests/test_orca/test_image_renderers.py | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/python/plotly/plotly/tests/test_orca/test_image_renderers.py b/packages/python/plotly/plotly/tests/test_orca/test_image_renderers.py index b5d3bf3ea2d..07f1490319e 100644 --- a/packages/python/plotly/plotly/tests/test_orca/test_image_renderers.py +++ b/packages/python/plotly/plotly/tests/test_orca/test_image_renderers.py @@ -49,11 +49,18 @@ def test_png_renderer_mimetype(fig1): expected = {"image/png": image_str} pio.renderers.render_on_display = False - assert fig1._repr_mimebundle_(None, None) is None + + with mock.patch("IPython.display.display") as mock_display: + fig1._ipython_display_() + + # assert fig1._repr_mimebundle_(None, None) is None + mock_display.assert_not_called() pio.renderers.render_on_display = True - bundle = fig1._repr_mimebundle_(None, None) - assert bundle == expected + with mock.patch("IPython.display.display") as mock_display: + fig1._ipython_display_() + + mock_display.assert_called_once_with(expected, raw=True) def test_svg_renderer_show(fig1): @@ -131,8 +138,15 @@ def test_mimetype_combination(fig1): expected = {"image/png": image_str, plotly_mimetype: plotly_mimetype_dict} pio.renderers.render_on_display = False - assert fig1._repr_mimebundle_(None, None) is None + + with mock.patch("IPython.display.display") as mock_display: + fig1._ipython_display_() + + # assert fig1._repr_mimebundle_(None, None) is None + mock_display.assert_not_called() pio.renderers.render_on_display = True - bundle = fig1._repr_mimebundle_(None, None) - assert bundle == expected + with mock.patch("IPython.display.display") as mock_display: + fig1._ipython_display_() + + mock_display.assert_called_once_with(expected, raw=True) From 7c1bbc2276e5a45111ea8bcc50cc3ddc4d94a983 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 19 Jun 2019 09:32:37 -0400 Subject: [PATCH 03/11] Have orca retry image export request on "522: client socket timeout" --- packages/python/plotly/plotly/io/_orca.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/python/plotly/plotly/io/_orca.py b/packages/python/plotly/plotly/io/_orca.py index 8fe3fdef784..be4af6f190c 100644 --- a/packages/python/plotly/plotly/io/_orca.py +++ b/packages/python/plotly/plotly/io/_orca.py @@ -1402,6 +1402,11 @@ def request_image_with_retrying(**kwargs): request_params = {k: v for k, v, in kwargs.items() if v is not None} json_str = json.dumps(request_params, cls=_plotly_utils.utils.PlotlyJSONEncoder) response = post(server_url + "/", data=json_str) + + if response.status_code == 522: + # Retry on "522: client socket timeout" + raise OSError("522: client socket timeout") + return response @@ -1546,6 +1551,7 @@ def to_image(fig, format=None, width=None, height=None, scale=None, validate=Tru # orca code base. # statusMsg: { # 400: 'invalid or malformed request syntax', + # 522: client socket timeout # 525: 'plotly.js error', # 526: 'plotly.js version 1.11.0 or up required', # 530: 'image conversion error' From 1a45c15822c06198d0f508abaa25ca9cfedccecc Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 19 Jun 2019 09:41:07 -0400 Subject: [PATCH 04/11] fix renderers test case --- .../plotly/tests/test_io/test_renderers.py | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/python/plotly/plotly/tests/test_io/test_renderers.py b/packages/python/plotly/plotly/tests/test_io/test_renderers.py index fddee0823c9..4e7f40da96c 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -43,11 +43,17 @@ def test_json_renderer_mimetype(fig1): expected = {"application/json": json.loads(pio.to_json(fig1, remove_uids=False))} pio.renderers.render_on_display = False - assert fig1._repr_mimebundle_(None, None) is None + + with mock.patch("IPython.display.display") as mock_display: + fig1._ipython_display_() + + mock_display.assert_not_called() pio.renderers.render_on_display = True - bundle = fig1._repr_mimebundle_(None, None) - assert bundle == expected + with mock.patch("IPython.display.display") as mock_display: + fig1._ipython_display_() + + mock_display.assert_called_once_with(expected, raw=True) def test_json_renderer_show(fig1): @@ -88,11 +94,17 @@ def test_plotly_mimetype_renderer_mimetype(fig1, renderer): expected[plotly_mimetype]["config"] = {"plotlyServerURL": "https://plot.ly"} pio.renderers.render_on_display = False - assert fig1._repr_mimebundle_(None, None) is None + + with mock.patch("IPython.display.display") as mock_display: + fig1._ipython_display_() + + mock_display.assert_not_called() pio.renderers.render_on_display = True - bundle = fig1._repr_mimebundle_(None, None) - assert bundle == expected + with mock.patch("IPython.display.display") as mock_display: + fig1._ipython_display_() + + mock_display.assert_called_once_with(expected, raw=True) @pytest.mark.parametrize("renderer", plotly_mimetype_renderers) From e59c3ebfaf66ba535e1ca32749656489c1063a3d Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 19 Jun 2019 09:54:11 -0400 Subject: [PATCH 05/11] Lengthen orca timeout and restart server on client socket timeout --- packages/python/plotly/plotly/io/_orca.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/python/plotly/plotly/io/_orca.py b/packages/python/plotly/plotly/io/_orca.py index be4af6f190c..5795daf3e80 100644 --- a/packages/python/plotly/plotly/io/_orca.py +++ b/packages/python/plotly/plotly/io/_orca.py @@ -1387,7 +1387,7 @@ def ensure_server(): orca_state["shutdown_timer"] = t -@retrying.retry(wait_random_min=5, wait_random_max=10, stop_max_delay=30000) +@retrying.retry(wait_random_min=5, wait_random_max=10, stop_max_delay=60000) def request_image_with_retrying(**kwargs): """ Helper method to perform an image request to a running orca server process @@ -1404,7 +1404,9 @@ def request_image_with_retrying(**kwargs): response = post(server_url + "/", data=json_str) if response.status_code == 522: - # Retry on "522: client socket timeout" + # On "522: client socket timeout", return server and keep trying + shutdown_server() + ensure_server() raise OSError("522: client socket timeout") return response From 27cea02ff5cf7bd67aacba3633f456178ea56469 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 19 Jun 2019 10:04:15 -0400 Subject: [PATCH 06/11] Retry _create_or_overwrite up to 5 times --- .../chart-studio/chart_studio/plotly/plotly.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/python/chart-studio/chart_studio/plotly/plotly.py b/packages/python/chart-studio/chart_studio/plotly/plotly.py index 95e16c982bf..8f8e89043bd 100644 --- a/packages/python/chart-studio/chart_studio/plotly/plotly.py +++ b/packages/python/chart-studio/chart_studio/plotly/plotly.py @@ -1445,7 +1445,7 @@ def get_grid(grid_url, raw=False): return Grid(parsed_content, fid) -def _create_or_overwrite(data, filetype): +def _create_or_overwrite(data, filetype, max_retries=5): """ Create or overwrite (if file exists) and grid, plot, spectacle, or dashboard object @@ -1495,8 +1495,14 @@ def _create_or_overwrite(data, filetype): pass # Create file - res = api_module.create(data) - res.raise_for_status() + try: + res = api_module.create(data) + res.raise_for_status() + except exceptions.PlotlyRequestError as e: + if max_retries > 0 and "already exists" in e.message: + # Retry _create_or_overwrite + time.sleep(1) + _create_or_overwrite(data, filetype, max_retries=max_retries - 1) # Get resulting file content file_info = res.json() From a78c31e11103cef052dbef6367f302e86cd19d4f Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 19 Jun 2019 11:00:22 -0400 Subject: [PATCH 07/11] Change name of is_share_key_included -> is_share_key_included2 to work around "An intermediate folder or the file specified by 'path' is non-unique!" error --- .../tests/test_plot_ly/test_plotly/test_plot.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/python/chart-studio/chart_studio/tests/test_plot_ly/test_plotly/test_plot.py b/packages/python/chart-studio/chart_studio/tests/test_plot_ly/test_plotly/test_plot.py index 9d4f73b1d99..282666d325f 100644 --- a/packages/python/chart-studio/chart_studio/tests/test_plot_ly/test_plotly/test_plot.py +++ b/packages/python/chart-studio/chart_studio/tests/test_plot_ly/test_plotly/test_plot.py @@ -166,7 +166,7 @@ def test_plot_url_given_sharing_key(self): self.simple_figure, validate ) kwargs = { - "filename": "is_share_key_included", + "filename": "is_share_key_included2", "world_readable": False, "auto_open": False, "sharing": "secret", @@ -182,7 +182,7 @@ def test_plot_url_response_given_sharing_key(self): # be 200 kwargs = { - "filename": "is_share_key_included", + "filename": "is_share_key_included2", "auto_open": False, "world_readable": False, "sharing": "secret", @@ -203,7 +203,7 @@ def test_private_plot_response_with_and_without_share_key(self): # share_key is added it should be 200 kwargs = { - "filename": "is_share_key_included", + "filename": "is_share_key_included2", "world_readable": False, "auto_open": False, "sharing": "private", From 2ce5cd06da2bf665b6456ca182bfb6ae10dbd5d6 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 19 Jun 2019 11:01:11 -0400 Subject: [PATCH 08/11] Only use the new _create_or_overwrite on grids, use the old create_or_update for figures --- .../chart_studio/plotly/plotly.py | 76 +++++++++++++++++-- 1 file changed, 71 insertions(+), 5 deletions(-) diff --git a/packages/python/chart-studio/chart_studio/plotly/plotly.py b/packages/python/chart-studio/chart_studio/plotly/plotly.py index 8f8e89043bd..d257d97509d 100644 --- a/packages/python/chart-studio/chart_studio/plotly/plotly.py +++ b/packages/python/chart-studio/chart_studio/plotly/plotly.py @@ -282,7 +282,7 @@ def plot(figure_or_data, validate=True, **plot_options): _set_grid_column_references(figure, grid) payload["figure"] = figure - file_info = _create_or_overwrite(payload, "plot") + file_info = _create_or_update(payload, "plot") # Compute viewing URL if sharing == "secret": @@ -1445,6 +1445,70 @@ def get_grid(grid_url, raw=False): return Grid(parsed_content, fid) +def _create_or_update(data, filetype): + """ + Create or update (if file exists) and grid, plot, spectacle, or dashboard + object + Parameters + ---------- + data: dict + update/create API payload + filetype: str + One of 'plot', 'grid', 'spectacle_presentation', or 'dashboard' + Returns + ------- + dict + File info from API response + """ + api_module = getattr(v2, filetype + "s") + + # lookup if pre-existing filename already exists + if "parent_path" in data: + filename = data["parent_path"] + "/" + data["filename"] + else: + filename = data.get("filename", None) + + if filename: + try: + lookup_res = v2.files.lookup(filename) + if isinstance(lookup_res.content, bytes): + content = lookup_res.content.decode("utf-8") + else: + content = lookup_res.content + + matching_file = json.loads(content) + + if matching_file["filetype"] == filetype: + fid = matching_file["fid"] + res = api_module.update(fid, data) + else: + raise _plotly_utils.exceptions.PlotlyError( + """ +'{filename}' is already a {other_filetype} in your account. +While you can overwrite {filetype}s with the same name, you can't overwrite +files with a different type. Try deleting '{filename}' in your account or +changing the filename.""".format( + filename=filename, + filetype=filetype, + other_filetype=matching_file["filetype"], + ) + ) + + except exceptions.PlotlyRequestError: + res = api_module.create(data) + else: + res = api_module.create(data) + + # Check response + res.raise_for_status() + + # Get resulting file content + file_info = res.json() + file_info = file_info.get("file", file_info) + + return file_info + + def _create_or_overwrite(data, filetype, max_retries=5): """ Create or overwrite (if file exists) and grid, plot, spectacle, @@ -1497,14 +1561,16 @@ def _create_or_overwrite(data, filetype, max_retries=5): # Create file try: res = api_module.create(data) - res.raise_for_status() except exceptions.PlotlyRequestError as e: if max_retries > 0 and "already exists" in e.message: # Retry _create_or_overwrite time.sleep(1) - _create_or_overwrite(data, filetype, max_retries=max_retries - 1) + return _create_or_overwrite(data, filetype, max_retries=max_retries - 1) + else: + raise # Get resulting file content + res.raise_for_status() file_info = res.json() file_info = file_info.get("file", file_info) @@ -1592,7 +1658,7 @@ def upload(cls, dashboard, filename, sharing="public", auto_open=True): "world_readable": world_readable, } - file_info = _create_or_overwrite(data, "dashboard") + file_info = _create_or_update(data, "dashboard") url = file_info["web_url"] @@ -1689,7 +1755,7 @@ def upload(cls, presentation, filename, sharing="public", auto_open=True): "world_readable": world_readable, } - file_info = _create_or_overwrite(data, "spectacle_presentation") + file_info = _create_or_update(data, "spectacle_presentation") url = file_info["web_url"] From f1a63b33c41844cd55c0b1a0f0ed5c301ffedad3 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 19 Jun 2019 11:25:46 -0400 Subject: [PATCH 09/11] Run chart studio tests sequentially --- .circleci/config.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 756ef1fedd7..15dda14a80b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -373,8 +373,9 @@ workflows: # 3.7 optional disabled due to current shapely incompatibility # - python-3.7-optional - python-2.7-plot_ly - - python-3.5-plot_ly - - python-3.7-plot_ly + - python-3.7-plot_ly: + requires: + - python-2.7-plot_ly - python-2-7-orca - python-3-5-orca - python-3-7-orca From 7d8db4ab5f0d7d26ff0551f87a58940e2c511e24 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 19 Jun 2019 11:26:09 -0400 Subject: [PATCH 10/11] rename stream-test chart studio file --- .../tests/test_plot_ly/test_stream/test_stream.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/python/chart-studio/chart_studio/tests/test_plot_ly/test_stream/test_stream.py b/packages/python/chart-studio/chart_studio/tests/test_plot_ly/test_stream/test_stream.py index 48f5b38c3e1..01f8a5b143a 100644 --- a/packages/python/chart-studio/chart_studio/tests/test_plot_ly/test_stream/test_stream.py +++ b/packages/python/chart-studio/chart_studio/tests/test_plot_ly/test_stream/test_stream.py @@ -36,7 +36,7 @@ def test_initialize_stream_plot(self): [Scatter(x=[], y=[], mode="markers", stream=stream)], auto_open=False, world_readable=True, - filename="stream-test", + filename="stream-test2", ) self.assertTrue(url.startswith("https://plot.ly/~PythonAPI/")) time.sleep(0.5) @@ -49,7 +49,7 @@ def test_stream_single_points(self): [Scatter(x=[], y=[], mode="markers", stream=stream)], auto_open=False, world_readable=True, - filename="stream-test", + filename="stream-test2", ) time.sleep(0.5) my_stream = py.Stream(tk) @@ -66,7 +66,7 @@ def test_stream_multiple_points(self): [Scatter(x=[], y=[], mode="markers", stream=stream)], auto_open=False, world_readable=True, - filename="stream-test", + filename="stream-test2", ) time.sleep(0.5) my_stream = py.Stream(tk) @@ -83,7 +83,7 @@ def test_stream_layout(self): [Scatter(x=[], y=[], mode="markers", stream=stream)], auto_open=False, world_readable=True, - filename="stream-test", + filename="stream-test2", ) time.sleep(0.5) title_0 = "some title i picked first" From 9ac189a2aa18bb1dde4dd0c1cb0e99e437fcae29 Mon Sep 17 00:00:00 2001 From: Jon Mease Date: Wed, 19 Jun 2019 12:03:22 -0400 Subject: [PATCH 11/11] Use grid "destory" rather than "trash"+"permanent delete" endpoints --- .../chart-studio/chart_studio/api/v2/grids.py | 12 ++++++++++++ .../chart-studio/chart_studio/plotly/plotly.py | 17 +++++++---------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/python/chart-studio/chart_studio/api/v2/grids.py b/packages/python/chart-studio/chart_studio/api/v2/grids.py index b91bb2e052d..505829d942a 100644 --- a/packages/python/chart-studio/chart_studio/api/v2/grids.py +++ b/packages/python/chart-studio/chart_studio/api/v2/grids.py @@ -95,6 +95,18 @@ def permanent_delete(fid): return request("delete", url) +def destroy(fid): + """ + Permanently delete a grid file from Plotly. + + :param (str) fid: The `{username}:{idlocal}` identifier. E.g. `foo:88`. + :returns: (requests.Response) Returns response directly from requests. + + """ + url = build_url(RESOURCE, id=fid) + return request("delete", url) + + def lookup(path, parent=None, user=None, exists=None): """ Retrieve a grid file from Plotly without needing a fid. diff --git a/packages/python/chart-studio/chart_studio/plotly/plotly.py b/packages/python/chart-studio/chart_studio/plotly/plotly.py index d257d97509d..7e1595e83cb 100644 --- a/packages/python/chart-studio/chart_studio/plotly/plotly.py +++ b/packages/python/chart-studio/chart_studio/plotly/plotly.py @@ -1094,7 +1094,7 @@ def upload( if parent_path != "": payload["parent_path"] = parent_path - file_info = _create_or_overwrite(payload, "grid") + file_info = _create_or_overwrite_grid(payload) cols = file_info["cols"] fid = file_info["fid"] @@ -1447,7 +1447,7 @@ def get_grid(grid_url, raw=False): def _create_or_update(data, filetype): """ - Create or update (if file exists) and grid, plot, spectacle, or dashboard + Create or update (if file exists) and plot, spectacle, or dashboard object Parameters ---------- @@ -1509,10 +1509,9 @@ def _create_or_update(data, filetype): return file_info -def _create_or_overwrite(data, filetype, max_retries=5): +def _create_or_overwrite_grid(data, max_retries=3): """ - Create or overwrite (if file exists) and grid, plot, spectacle, - or dashboard object + Create or overwrite (if file exists) a grid Parameters ---------- @@ -1526,7 +1525,7 @@ def _create_or_overwrite(data, filetype, max_retries=5): dict File info from API response """ - api_module = getattr(v2, filetype + "s") + api_module = v2.grids # lookup if pre-existing filename already exists if "parent_path" in data: @@ -1548,11 +1547,9 @@ def _create_or_overwrite(data, filetype, max_retries=5): # Delete fid # This requires sending file to trash and then deleting it - res = api_module.trash(fid) + res = api_module.destroy(fid) res.raise_for_status() - res = api_module.permanent_delete(fid) - res.raise_for_status() except exceptions.PlotlyRequestError as e: # Raise on trash or permanent delete # Pass through to try creating the file anyway @@ -1565,7 +1562,7 @@ def _create_or_overwrite(data, filetype, max_retries=5): if max_retries > 0 and "already exists" in e.message: # Retry _create_or_overwrite time.sleep(1) - return _create_or_overwrite(data, filetype, max_retries=max_retries - 1) + return _create_or_overwrite_grid(data, max_retries=max_retries - 1) else: raise