From a547e370c05bfde58b11db2279c08cca7812d779 Mon Sep 17 00:00:00 2001 From: Nicolas Kruchten Date: Wed, 21 Jul 2021 13:55:53 -0400 Subject: [PATCH 1/5] improve browser error messages --- CHANGELOG.md | 1 + .../plotly/plotly/io/_base_renderers.py | 26 ++++++++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66306d3ce5..62f9af32f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Fixed regression introduced in version 5.0.0 where pandas/numpy arrays with `dtype` of Object were being converted to `list` values when added to a Figure ([#3292](https://github.com/plotly/plotly.py/issues/3292), [#3293](https://github.com/plotly/plotly.py/pull/3293)) + - Better detection of Chrome and Chromium browsers in the Renderers framework, especially on Linux ([#3278](https://github.com/plotly/plotly.py/pull/3278)) with thanks to [@c-chaitanya](https://github.com/c-chaitanya) for the contribution ## [5.1.0] - 2021-06-28 diff --git a/packages/python/plotly/plotly/io/_base_renderers.py b/packages/python/plotly/plotly/io/_base_renderers.py index 66befb8bec..7f1f8e7f25 100644 --- a/packages/python/plotly/plotly/io/_base_renderers.py +++ b/packages/python/plotly/plotly/io/_base_renderers.py @@ -669,15 +669,21 @@ def open_html_in_browser(html, using=None, new=0, autoraise=True): if isinstance(html, six.string_types): html = html.encode("utf8") - if isinstance(using, tuple): - try: - using = [i for i in webbrowser._browsers.keys() if i in using][0] - except IndexError: - raise ValueError( - """ -Unable to find the given browser. -Try one among the following 'chrome', 'chromium', 'firefox' or 'default' """ - ) + browser = None + + if using is None: + browser = webbrowser.get() + else: + if not isinstance(using, tuple): + using = (using,) + for browser_key in using: + try: + browser = webbrowser.get(browser_key) + except webbrowser.Error: + pass + + if browser is None: + raise ValueError("Can't locate a browser with key in " + str(using)) class OneShotRequestHandler(BaseHTTPRequestHandler): def do_GET(self): @@ -694,7 +700,7 @@ def log_message(self, format, *args): pass server = HTTPServer(("127.0.0.1", 0), OneShotRequestHandler) - webbrowser.get(using).open( + browser.open( "http://127.0.0.1:%s" % server.server_port, new=new, autoraise=autoraise ) From e7a3809a19db9f4367504ecdb54c40c9b1770fb7 Mon Sep 17 00:00:00 2001 From: Nicolas Kruchten Date: Wed, 21 Jul 2021 13:57:54 -0400 Subject: [PATCH 2/5] revert test change --- packages/python/plotly/plotly/tests/test_io/test_renderers.py | 2 -- 1 file changed, 2 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 654929ba2c..44867e3e8f 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -225,8 +225,6 @@ def test_notebook_connected_show(fig1, name, connected): def test_browser_renderer_show(fig1, renderer): pio.renderers.default = renderer renderer_obj = pio.renderers[renderer] - # scan through webbrowser._browsers.keys() and assign the browser name registered with os - renderer_obj.using = [i for i in webbrowser._browsers.keys() if renderer in i][0] # Setup mocks mock_get = MagicMock(name="test get") From 72a949c417c16a5d6044ed347f3972c1269f074c Mon Sep 17 00:00:00 2001 From: Nicolas Kruchten Date: Wed, 21 Jul 2021 14:05:42 -0400 Subject: [PATCH 3/5] fix test --- .../python/plotly/plotly/tests/test_io/test_renderers.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 44867e3e8f..426bcf7b45 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -249,7 +249,11 @@ def open_url(url, new=0, autoraise=True): pio.show(fig1) # check get args - mock_get.assert_called_once_with(renderer_obj.using) + if isinstance(renderer_obj.using, tuple): + for using in renderer_obj.using: + mock_get.assert_called_with(using) + else: + mock_get.assert_called_once_with(renderer_obj.using) # check open args mock_call_args = mock_browser.open.call_args From 458554a4a82e50b24ce5f6024dead6043abed499 Mon Sep 17 00:00:00 2001 From: Nicolas Kruchten Date: Wed, 21 Jul 2021 14:12:43 -0400 Subject: [PATCH 4/5] tweak tests --- packages/python/plotly/plotly/io/_base_renderers.py | 2 +- .../plotly/plotly/tests/test_io/test_renderers.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/python/plotly/plotly/io/_base_renderers.py b/packages/python/plotly/plotly/io/_base_renderers.py index 7f1f8e7f25..5fe0466dbc 100644 --- a/packages/python/plotly/plotly/io/_base_renderers.py +++ b/packages/python/plotly/plotly/io/_base_renderers.py @@ -672,7 +672,7 @@ def open_html_in_browser(html, using=None, new=0, autoraise=True): browser = None if using is None: - browser = webbrowser.get() + browser = webbrowser.get(None) else: if not isinstance(using, tuple): using = (using,) 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 426bcf7b45..1cfe4d558f 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -221,10 +221,14 @@ def test_notebook_connected_show(fig1, name, connected): # Browser # ------- -@pytest.mark.parametrize("renderer", ["browser", "chrome", "firefox"]) +@pytest.mark.parametrize("renderer", ["browser", "chrome", "chromium", "firefox"]) def test_browser_renderer_show(fig1, renderer): pio.renderers.default = renderer renderer_obj = pio.renderers[renderer] + using = renderer_obj.using + if isinstance(renderer_obj.using, tuple): + using = renderer_obj.using[0] + renderer_obj.using = using # Setup mocks mock_get = MagicMock(name="test get") @@ -249,11 +253,7 @@ def open_url(url, new=0, autoraise=True): pio.show(fig1) # check get args - if isinstance(renderer_obj.using, tuple): - for using in renderer_obj.using: - mock_get.assert_called_with(using) - else: - mock_get.assert_called_once_with(renderer_obj.using) + mock_get.assert_called_once_with(using) # check open args mock_call_args = mock_browser.open.call_args From 1e24ca002b7936d5fe9ce7798d2e47c371993dd7 Mon Sep 17 00:00:00 2001 From: Nicolas Kruchten Date: Wed, 21 Jul 2021 14:18:58 -0400 Subject: [PATCH 5/5] better tests --- packages/python/plotly/plotly/io/_base_renderers.py | 2 ++ .../python/plotly/plotly/tests/test_io/test_renderers.py | 7 +++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/python/plotly/plotly/io/_base_renderers.py b/packages/python/plotly/plotly/io/_base_renderers.py index 5fe0466dbc..18d6ba1855 100644 --- a/packages/python/plotly/plotly/io/_base_renderers.py +++ b/packages/python/plotly/plotly/io/_base_renderers.py @@ -679,6 +679,8 @@ def open_html_in_browser(html, using=None, new=0, autoraise=True): for browser_key in using: try: browser = webbrowser.get(browser_key) + if browser is not None: + break except webbrowser.Error: pass 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 1cfe4d558f..bb521621c0 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -226,9 +226,8 @@ def test_browser_renderer_show(fig1, renderer): pio.renderers.default = renderer renderer_obj = pio.renderers[renderer] using = renderer_obj.using - if isinstance(renderer_obj.using, tuple): - using = renderer_obj.using[0] - renderer_obj.using = using + if not isinstance(renderer_obj.using, tuple): + using = (using,) # Setup mocks mock_get = MagicMock(name="test get") @@ -253,7 +252,7 @@ def open_url(url, new=0, autoraise=True): pio.show(fig1) # check get args - mock_get.assert_called_once_with(using) + mock_get.assert_any_call(using[0]) # check open args mock_call_args = mock_browser.open.call_args