V4: Have side-effect renderers show figure on display (#1629)

* 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.

* Fix renderer tests

* Have orca retry image export request on "522: client socket timeout"

* fix renderers test case

* 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

* Only use the new _create_or_overwrite on grids, use the old
create_or_update for figures

* Run chart studio tests sequentially

* Use grid "destory" rather than "trash"+"permanent delete" endpoints

Author: jonmmease

.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

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.

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":
@@ -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"]
@@ -1445,10 +1445,73 @@ def get_grid(grid_url, raw=False):
     return Grid(parsed_content, fid)
 
 
-def _create_or_overwrite(data, filetype):
+def _create_or_update(data, filetype):
     """
-    Create or overwrite (if file exists) and grid, plot, spectacle,
-    or dashboard object
+    Create or update (if file exists) and 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_grid(data, max_retries=3):
+    """
+    Create or overwrite (if file exists) a grid
 
     Parameters
     ----------
@@ -1462,7 +1525,7 @@ def _create_or_overwrite(data, filetype):
     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:
@@ -1484,21 +1547,27 @@ def _create_or_overwrite(data, filetype):
 
             # 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
             pass
 
     # Create file
-    res = api_module.create(data)
-    res.raise_for_status()
+    try:
+        res = api_module.create(data)
+    except exceptions.PlotlyRequestError as e:
+        if max_retries > 0 and "already exists" in e.message:
+            # Retry _create_or_overwrite
+            time.sleep(1)
+            return _create_or_overwrite_grid(data, 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)
 
@@ -1586,7 +1655,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"]
 
@@ -1683,7 +1752,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"]
 

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",

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"

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):
         """

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):

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
@@ -1402,6 +1402,13 @@ 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:
+        # On "522: client socket timeout", return server and keep trying
+        shutdown_server()
+        ensure_server()
+        raise OSError("522: client socket timeout")
+
     return response
 
 
@@ -1546,6 +1553,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'

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)