From 57ec19f909539202e4f9b139d43823ad789c9395 Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Sat, 15 Apr 2023 12:31:30 +0200 Subject: [PATCH 01/11] Be more forgiving when anything with opening a webbrowser fails --- packages/python/plotly/plotly/io/_renderers.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/python/plotly/plotly/io/_renderers.py b/packages/python/plotly/plotly/io/_renderers.py index 3d0dd979c89..347f19f4d8b 100644 --- a/packages/python/plotly/plotly/io/_renderers.py +++ b/packages/python/plotly/plotly/io/_renderers.py @@ -525,13 +525,14 @@ def show(fig, renderer=None, validate=True, **kwargs): else: # If ipython isn't available, try to display figures in the default # browser - import webbrowser - try: + import webbrowser webbrowser.get() default_renderer = "browser" - except webbrowser.Error: - # Default browser could not be loaded + except Exception: + # Many things could have gone wrong + # There could not be a webbrowser Python module, + # or the module may be a dumb placeholder pass renderers.render_on_display = True From d296c16b22edec2beb5da79c1025257d964422da Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Sun, 16 Apr 2023 08:52:14 +0000 Subject: [PATCH 02/11] Apply black --- packages/python/plotly/plotly/io/_renderers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/python/plotly/plotly/io/_renderers.py b/packages/python/plotly/plotly/io/_renderers.py index 347f19f4d8b..cf2434036f8 100644 --- a/packages/python/plotly/plotly/io/_renderers.py +++ b/packages/python/plotly/plotly/io/_renderers.py @@ -527,11 +527,12 @@ def show(fig, renderer=None, validate=True, **kwargs): # browser try: import webbrowser + webbrowser.get() default_renderer = "browser" except Exception: # Many things could have gone wrong - # There could not be a webbrowser Python module, + # There could not be a webbrowser Python module, # or the module may be a dumb placeholder pass From 19338856b889496da8a84c37dbbc1cadae3f5d94 Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Tue, 25 Apr 2023 12:36:26 +0200 Subject: [PATCH 03/11] Add tests for this scenario --- .../plotly/tests/test_io/test_renderers.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) 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 429c3178cc8..572bf2cbe1a 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -381,3 +381,24 @@ def test_repr_mimebundle_mixed_renderer(fig1): assert set(fig1._repr_mimebundle_().keys()) == set( {"application/vnd.plotly.v1+json", "text/html"} ) + + +def test_missing_webbrowser_module(fig1): + """ + Assert that no errors occur if the webbrowser module is absent + """ + removed_webbrowser_module = sys.modules['webbrowser'] + del sys.modules['webbrowser'] + fig1._repr_html_() + sys.modules['webbrowser'] = removed_webbrowser_module # restore everything after this test + + +def test_missing_webbrowser_methods(fig1): + """ + Assert that no errors occur if the webbrowser module does not contain some methods + """ + import webbrowser + removed_webbrowser_get_method = webbrowser.get + del webbrowser.get + fig1._repr_html_() + webbrowser.get = removed_webbrowser_get_method # restore everything after this test From b7a275dc0826e235f8d32d67beaf66b5464a2cd5 Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Tue, 25 Apr 2023 12:48:01 +0200 Subject: [PATCH 04/11] Run black --- .../plotly/plotly/tests/test_io/test_renderers.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 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 572bf2cbe1a..faefc2b71b2 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -126,6 +126,7 @@ def test_plotly_mimetype_renderer_show(fig1, renderer): # ------------ # See plotly/tests/test_orca/test_image_renderers.py + # HTML # ---- def assert_full_html(html): @@ -387,10 +388,12 @@ def test_missing_webbrowser_module(fig1): """ Assert that no errors occur if the webbrowser module is absent """ - removed_webbrowser_module = sys.modules['webbrowser'] - del sys.modules['webbrowser'] + removed_webbrowser_module = sys.modules["webbrowser"] + del sys.modules["webbrowser"] fig1._repr_html_() - sys.modules['webbrowser'] = removed_webbrowser_module # restore everything after this test + sys.modules[ + "webbrowser" + ] = removed_webbrowser_module # restore everything after this test def test_missing_webbrowser_methods(fig1): @@ -398,6 +401,7 @@ def test_missing_webbrowser_methods(fig1): Assert that no errors occur if the webbrowser module does not contain some methods """ import webbrowser + removed_webbrowser_get_method = webbrowser.get del webbrowser.get fig1._repr_html_() From f6af929742f3b48d5b0edb54f065ca93b4777aee Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Fri, 5 May 2023 20:37:42 +0200 Subject: [PATCH 05/11] Update packages/python/plotly/plotly/tests/test_io/test_renderers.py Co-authored-by: Alex Johnson --- .../python/plotly/plotly/tests/test_io/test_renderers.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 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 faefc2b71b2..fa02e083799 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -403,6 +403,9 @@ def test_missing_webbrowser_methods(fig1): import webbrowser removed_webbrowser_get_method = webbrowser.get - del webbrowser.get - fig1._repr_html_() - webbrowser.get = removed_webbrowser_get_method # restore everything after this test + try: + del webbrowser.get + fig1._repr_html_() + finally: + # restore everything after this test + webbrowser.get = removed_webbrowser_get_method From e2ec195fe4e9d619190999715f975a54d39683d8 Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Sat, 3 Jun 2023 16:59:13 +0200 Subject: [PATCH 06/11] Mock absent webbrowser module --- .../python/plotly/plotly/tests/test_io/test_renderers.py | 8 ++------ 1 file changed, 2 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 fa02e083799..daead1fa274 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -388,12 +388,8 @@ def test_missing_webbrowser_module(fig1): """ Assert that no errors occur if the webbrowser module is absent """ - removed_webbrowser_module = sys.modules["webbrowser"] - del sys.modules["webbrowser"] - fig1._repr_html_() - sys.modules[ - "webbrowser" - ] = removed_webbrowser_module # restore everything after this test + with mock.patch("builtins.__import__", side_effect=ImportError): + fig1._repr_html_() def test_missing_webbrowser_methods(fig1): From 2248f1d6960f74b30c4f2054ec232ca3a4670719 Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Sat, 3 Jun 2023 16:59:13 +0200 Subject: [PATCH 07/11] Revert "Mock absent webbrowser module" This reverts commit e2ec195fe4e9d619190999715f975a54d39683d8. --- .../python/plotly/plotly/tests/test_io/test_renderers.py | 8 ++++++-- 1 file changed, 6 insertions(+), 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 daead1fa274..fa02e083799 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -388,8 +388,12 @@ def test_missing_webbrowser_module(fig1): """ Assert that no errors occur if the webbrowser module is absent """ - with mock.patch("builtins.__import__", side_effect=ImportError): - fig1._repr_html_() + removed_webbrowser_module = sys.modules["webbrowser"] + del sys.modules["webbrowser"] + fig1._repr_html_() + sys.modules[ + "webbrowser" + ] = removed_webbrowser_module # restore everything after this test def test_missing_webbrowser_methods(fig1): From 855f6225b072d250f3c90ce086e43f8d3bb2743a Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Sat, 3 Jun 2023 17:32:33 +0200 Subject: [PATCH 08/11] This should crash the test --- packages/python/plotly/plotly/tests/test_io/test_renderers.py | 1 + 1 file changed, 1 insertion(+) 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 fa02e083799..44fdc57c0f5 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -390,6 +390,7 @@ def test_missing_webbrowser_module(fig1): """ removed_webbrowser_module = sys.modules["webbrowser"] del sys.modules["webbrowser"] + import webbrowser fig1._repr_html_() sys.modules[ "webbrowser" From 7965119a78c19f0ed1fe9ab24b911c6e7280c41f Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Sat, 3 Jun 2023 17:34:17 +0200 Subject: [PATCH 09/11] Apply suggestion --- packages/python/plotly/plotly/tests/test_io/test_renderers.py | 3 ++- 1 file changed, 2 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 44fdc57c0f5..950ff8032a9 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -390,7 +390,8 @@ def test_missing_webbrowser_module(fig1): """ removed_webbrowser_module = sys.modules["webbrowser"] del sys.modules["webbrowser"] - import webbrowser + with pytest.raises(ImportError): + import webbrowser fig1._repr_html_() sys.modules[ "webbrowser" From 74968c1f0c70074618f39d24e23c3f03a67b0033 Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Sat, 3 Jun 2023 17:43:53 +0200 Subject: [PATCH 10/11] Temporarily make webbrowser module absent the right way --- .../plotly/tests/test_io/test_renderers.py | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 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 950ff8032a9..ce0c59df2a7 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -388,14 +388,22 @@ def test_missing_webbrowser_module(fig1): """ Assert that no errors occur if the webbrowser module is absent """ - removed_webbrowser_module = sys.modules["webbrowser"] - del sys.modules["webbrowser"] - with pytest.raises(ImportError): - import webbrowser - fig1._repr_html_() - sys.modules[ - "webbrowser" - ] = removed_webbrowser_module # restore everything after this test + try: + import builtins + except ImportError: + import __builtin__ as builtins + realimport = builtins.__import__ + + def webbrowser_absent_import(name, globals, locals, fromlist, level): + """ + Mimick an absent webbrowser module + """ + if name == 'webbrowser': + raise ImportError + return realimport(name, globals, locals, fromlist, level) + + with mock.patch("builtins.__import__", webbrowser_absent_import): + fig1._repr_html_() def test_missing_webbrowser_methods(fig1): From cdc7ffdc6aa021eeea9efb69f7c56d186870ab2d Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Sat, 3 Jun 2023 17:51:36 +0200 Subject: [PATCH 11/11] Run black --- .../python/plotly/plotly/tests/test_io/test_renderers.py | 9 +++++++-- 1 file changed, 7 insertions(+), 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 ce0c59df2a7..13b2494306d 100644 --- a/packages/python/plotly/plotly/tests/test_io/test_renderers.py +++ b/packages/python/plotly/plotly/tests/test_io/test_renderers.py @@ -398,11 +398,16 @@ def webbrowser_absent_import(name, globals, locals, fromlist, level): """ Mimick an absent webbrowser module """ - if name == 'webbrowser': + if name == "webbrowser": raise ImportError return realimport(name, globals, locals, fromlist, level) - + with mock.patch("builtins.__import__", webbrowser_absent_import): + # 1: check whether importing webbrowser actually results in an ImportError + with pytest.raises(ImportError): + import webbrowser + + # 2: check whether the _repr_html_ can handle it regardless fig1._repr_html_()