diff --git a/packages/python/plotly/plotly/basedatatypes.py b/packages/python/plotly/plotly/basedatatypes.py index 7954dea37f6..e39fe0fdcce 100644 --- a/packages/python/plotly/plotly/basedatatypes.py +++ b/packages/python/plotly/plotly/basedatatypes.py @@ -3578,7 +3578,7 @@ def write_html(self, *args, **kwargs): ---------- file: str or writeable A string representing a local file path or a writeable object - (e.g. an open file descriptor) + (e.g. a pathlib.Path object or an open file descriptor) config: dict or None (default None) Plotly.js figure config options auto_play: bool (default=True) @@ -3751,7 +3751,7 @@ def write_image(self, *args, **kwargs): ---------- file: str or writeable A string representing a local file path or a writeable object - (e.g. an open file descriptor) + (e.g. a pathlib.Path object or an open file descriptor) format: str or None The desired image format. One of diff --git a/packages/python/plotly/plotly/io/_html.py b/packages/python/plotly/plotly/io/_html.py index cb90de32f98..ce5534faf10 100644 --- a/packages/python/plotly/plotly/io/_html.py +++ b/packages/python/plotly/plotly/io/_html.py @@ -1,6 +1,7 @@ import uuid import json import os +from pathlib import Path import webbrowser import six @@ -401,7 +402,7 @@ def write_html( Figure object or dict representing a figure file: str or writeable A string representing a local file path or a writeable object - (e.g. an open file descriptor) + (e.g. a pathlib.Path object or an open file descriptor) config: dict or None (default None) Plotly.js figure config options auto_play: bool (default=True) @@ -520,24 +521,31 @@ def write_html( ) # Check if file is a string - file_is_str = isinstance(file, six.string_types) + if isinstance(file, six.string_types): + # Use the standard pathlib constructor to make a pathlib object. + path = Path(file) + elif isinstance(file, Path): # PurePath is the most general pathlib object. + # `file` is already a pathlib object. + path = file + else: + # We could not make a pathlib object out of file. Either `file` is an open file + # descriptor with a `write()` method or it's an invalid object. + path = None # Write HTML string - if file_is_str: - with open(file, "w") as f: - f.write(html_str) + if path is not None: + path.write_text(html_str) else: file.write(html_str) # Check if we should copy plotly.min.js to output directory - if file_is_str and full_html and include_plotlyjs == "directory": - bundle_path = os.path.join(os.path.dirname(file), "plotly.min.js") + if path is not None and full_html and include_plotlyjs == "directory": + bundle_path = path.parent / "plotly.min.js" - if not os.path.exists(bundle_path): - with open(bundle_path, "w") as f: - f.write(get_plotlyjs()) + if not bundle_path.exists(): + bundle_path.write_text(get_plotlyjs()) # Handle auto_open - if file_is_str and full_html and auto_open: - url = "file://" + os.path.abspath(file) + if path is not None and full_html and auto_open: + url = path.absolute().as_uri() webbrowser.open(url) diff --git a/packages/python/plotly/plotly/io/_json.py b/packages/python/plotly/plotly/io/_json.py index f67dbab3eb6..a4ab112409d 100644 --- a/packages/python/plotly/plotly/io/_json.py +++ b/packages/python/plotly/plotly/io/_json.py @@ -2,7 +2,7 @@ from six import string_types import json - +from pathlib import Path from plotly.io._utils import validate_coerce_fig_to_dict, validate_coerce_output_type @@ -68,7 +68,7 @@ def write_json(fig, file, validate=True, pretty=False, remove_uids=True): file: str or writeable A string representing a local file path or a writeable object - (e.g. an open file descriptor) + (e.g. a pathlib.Path object or an open file descriptor) pretty: bool (default False) True if JSON representation should be pretty-printed, False if @@ -87,17 +87,40 @@ def write_json(fig, file, validate=True, pretty=False, remove_uids=True): # Pass through validate argument and let to_json handle validation logic json_str = to_json(fig, validate=validate, pretty=pretty, remove_uids=remove_uids) - # Check if file is a string - # ------------------------- - file_is_str = isinstance(file, string_types) + # Try to cast `file` as a pathlib object `path`. + # ---------------------------------------------- + if isinstance(file, string_types): + # Use the standard Path constructor to make a pathlib object. + path = Path(file) + elif isinstance(file, Path): + # `file` is already a Path object. + path = file + else: + # We could not make a Path object out of file. Either `file` is an open file + # descriptor with a `write()` method or it's an invalid object. + path = None # Open file # --------- - if file_is_str: - with open(file, "w") as f: - f.write(json_str) + if path is None: + # We previously failed to make sense of `file` as a pathlib object. + # Attempt to write to `file` as an open file descriptor. + try: + file.write(json_str) + return + except AttributeError: + pass + raise ValueError( + """ +The 'file' argument '{file}' is not a string, pathlib.Path object, or file descriptor. +""".format( + file=file + ) + ) else: - file.write(json_str) + # We previously succeeded in interpreting `file` as a pathlib object. + # Now we can use `write_bytes()`. + path.write_text(json_str) def from_json(value, output_type="Figure", skip_invalid=False): @@ -162,7 +185,7 @@ def read_json(file, output_type="Figure", skip_invalid=False): ---------- file: str or readable A string containing the path to a local file or a read-able Python - object (e.g. an open file descriptor) + object (e.g. a pathlib.Path object or an open file descriptor) output_type: type or str (default 'Figure') The output figure type or type name. @@ -177,17 +200,25 @@ def read_json(file, output_type="Figure", skip_invalid=False): Figure or FigureWidget """ - # Check if file is a string + # Try to cast `file` as a pathlib object `path`. # ------------------------- - # If it's a string we assume it's a local file path. If it's not a string - # then we assume it's a read-able Python object + # ---------------------------------------------- file_is_str = isinstance(file, string_types) + if isinstance(file, string_types): + # Use the standard Path constructor to make a pathlib object. + path = Path(file) + elif isinstance(file, Path): + # `file` is already a Path object. + path = file + else: + # We could not make a Path object out of file. Either `file` is an open file + # descriptor with a `write()` method or it's an invalid object. + path = None # Read file contents into JSON string # ----------------------------------- - if file_is_str: - with open(file, "r") as f: - json_str = f.read() + if path is not None: + json_str = path.read_text() else: json_str = file.read() diff --git a/packages/python/plotly/plotly/io/_kaleido.py b/packages/python/plotly/plotly/io/_kaleido.py index e56d095d977..d7264bd06f6 100644 --- a/packages/python/plotly/plotly/io/_kaleido.py +++ b/packages/python/plotly/plotly/io/_kaleido.py @@ -2,6 +2,7 @@ from six import string_types import os import json +from pathlib import Path import plotly from plotly.io._utils import validate_coerce_fig_to_dict @@ -169,7 +170,7 @@ def write_image( file: str or writeable A string representing a local file path or a writeable object - (e.g. an open file descriptor) + (e.g. a pathlib.Path object or an open file descriptor) format: str or None The desired image format. One of @@ -228,14 +229,23 @@ def write_image( ------- None """ - # Check if file is a string - # ------------------------- - file_is_str = isinstance(file, string_types) + # Try to cast `file` as a pathlib object `path`. + # ---------------------------------------------- + if isinstance(file, string_types): + # Use the standard Path constructor to make a pathlib object. + path = Path(file) + elif isinstance(file, Path): + # `file` is already a Path object. + path = file + else: + # We could not make a Path object out of file. Either `file` is an open file + # descriptor with a `write()` method or it's an invalid object. + path = None # Infer format if not specified # ----------------------------- - if file_is_str and format is None: - _, ext = os.path.splitext(file) + if path is not None and format is None: + ext = path.suffix if ext: format = ext.lstrip(".") else: @@ -267,11 +277,25 @@ def write_image( # Open file # --------- - if file_is_str: - with open(file, "wb") as f: - f.write(img_data) + if path is None: + # We previously failed to make sense of `file` as a pathlib object. + # Attempt to write to `file` as an open file descriptor. + try: + file.write(img_data) + return + except AttributeError: + pass + raise ValueError( + """ +The 'file' argument '{file}' is not a string, pathlib.Path object, or file descriptor. +""".format( + file=file + ) + ) else: - file.write(img_data) + # We previously succeeded in interpreting `file` as a pathlib object. + # Now we can use `write_bytes()`. + path.write_bytes(img_data) def full_figure_for_development(fig, warn=True, as_dict=False): diff --git a/packages/python/plotly/plotly/io/_orca.py b/packages/python/plotly/plotly/io/_orca.py index 63152739f17..0b81b56844e 100644 --- a/packages/python/plotly/plotly/io/_orca.py +++ b/packages/python/plotly/plotly/io/_orca.py @@ -10,6 +10,7 @@ import warnings from copy import copy from contextlib import contextmanager +from pathlib import Path import tenacity from six import string_types @@ -1695,7 +1696,7 @@ def write_image( file: str or writeable A string representing a local file path or a writeable object - (e.g. an open file descriptor) + (e.g. a pathlib.Path object or an open file descriptor) format: str or None The desired image format. One of @@ -1741,16 +1742,25 @@ def write_image( None """ - # Check if file is a string - # ------------------------- - file_is_str = isinstance(file, string_types) + # Try to cast `file` as a pathlib object `path`. + # ---------------------------------------------- + if isinstance(file, string_types): + # Use the standard Path constructor to make a pathlib object. + path = Path(file) + elif isinstance(file, Path): + # `file` is already a Path object. + path = file + else: + # We could not make a Path object out of file. Either `file` is an open file + # descriptor with a `write()` method or it's an invalid object. + path = None # Infer format if not specified # ----------------------------- - if file_is_str and format is None: - _, ext = os.path.splitext(file) + if path is not None and format is None: + ext = path.suffix if ext: - format = validate_coerce_format(ext) + format = ext.lstrip(".") else: raise ValueError( """ @@ -1774,8 +1784,22 @@ def write_image( # Open file # --------- - if file_is_str: - with open(file, "wb") as f: - f.write(img_data) + if path is None: + # We previously failed to make sense of `file` as a pathlib object. + # Attempt to write to `file` as an open file descriptor. + try: + file.write(img_data) + return + except AttributeError: + pass + raise ValueError( + """ +The 'file' argument '{file}' is not a string, pathlib.Path object, or file descriptor. +""".format( + file=file + ) + ) else: - file.write(img_data) + # We previously succeeded in interpreting `file` as a pathlib object. + # Now we can use `write_bytes()`. + path.write_bytes(img_data) diff --git a/packages/python/plotly/plotly/tests/test_io/test_pathlib.py b/packages/python/plotly/plotly/tests/test_io/test_pathlib.py new file mode 100644 index 00000000000..48eab2bb9e5 --- /dev/null +++ b/packages/python/plotly/plotly/tests/test_io/test_pathlib.py @@ -0,0 +1,57 @@ +"""Test compatibility with pathlib.Path. + +See also relevant tests in + packages/python/plotly/plotly/tests/test_optional/test_kaleido/test_kaleido.py +""" + +from unittest import mock +import plotly.io as pio +from io import StringIO +from pathlib import Path +import re +from unittest.mock import Mock + +fig = {"layout": {"title": {"text": "figure title"}}} + + +def test_write_html(): + """Verify that various methods for producing HTML have equivalent results. + + The results will not be identical because the div id is pseudorandom. Thus + we compare the results after replacing the div id. + + We test the results of + - pio.to_html + - pio.write_html with a StringIO buffer + - pio.write_html with a mock pathlib Path + - pio.write_html with a mock file descriptor + """ + # Test pio.to_html + html = pio.to_html(fig) + + # Test pio.write_html with a StringIO buffer + sio = StringIO() + pio.write_html(fig, sio) + sio.seek(0) # Rewind to the beginning of the buffer, otherwise read() returns ''. + sio_html = sio.read() + assert replace_div_id(html) == replace_div_id(sio_html) + + # Test pio.write_html with a mock pathlib Path + mock_pathlib_path = Mock(spec=Path) + pio.write_html(fig, mock_pathlib_path) + mock_pathlib_path.write_text.assert_called_once() + (pl_html,) = mock_pathlib_path.write_text.call_args[0] + assert replace_div_id(html) == replace_div_id(pl_html) + + # Test pio.write_html with a mock file descriptor + mock_file_descriptor = Mock() + del mock_file_descriptor.write_bytes + pio.write_html(fig, mock_file_descriptor) + mock_file_descriptor.write.assert_called_once() + (fd_html,) = mock_file_descriptor.write.call_args[0] + assert replace_div_id(html) == replace_div_id(fd_html) + + +def replace_div_id(s): + uuid = re.search(r'
= (3, 3): - from unittest.mock import MagicMock - import tempfile -else: - from mock import MagicMock - from backports import tempfile +import tempfile +from unittest.mock import MagicMock +from pathlib import Path # fixtures @@ -154,6 +149,7 @@ def test_from_json_skip_invalid(fig1): def test_read_json_from_filelike(fig1, fig_type_spec, fig_type): # Configure file-like mock filemock = MagicMock() + del filemock.read_text filemock.read.return_value = pio.to_json(fig1) # read_json on mock file @@ -166,6 +162,30 @@ def test_read_json_from_filelike(fig1, fig_type_spec, fig_type): assert pio.to_json(fig1_loaded) == pio.to_json(fig1.to_dict()) +@pytest.mark.parametrize( + "fig_type_spec,fig_type", + [ + ("Figure", go.Figure), + (go.Figure, go.Figure), + ("FigureWidget", go.FigureWidget), + (go.FigureWidget, go.FigureWidget), + ], +) +def test_read_json_from_pathlib(fig1, fig_type_spec, fig_type): + # Configure pathlib.Path-like mock + filemock = MagicMock(spec=Path) + filemock.read_text.return_value = pio.to_json(fig1) + + # read_json on mock file + fig1_loaded = pio.read_json(filemock, output_type=fig_type_spec) + + # Check return type + assert isinstance(fig1_loaded, fig_type) + + # Check loaded figure + assert pio.to_json(fig1_loaded) == pio.to_json(fig1.to_dict()) + + @pytest.mark.parametrize( "fig_type_spec,fig_type", [ @@ -200,6 +220,7 @@ def test_read_json_from_file_string(fig1, fig_type_spec, fig_type): def test_write_json_filelike(fig1, pretty, remove_uids): # Configure file-like mock filemock = MagicMock() + del filemock.write_text # write_json to mock file pio.write_json(fig1, filemock, pretty=pretty, remove_uids=remove_uids) @@ -209,6 +230,20 @@ def test_write_json_filelike(fig1, pretty, remove_uids): filemock.write.assert_called_once_with(expected) +@pytest.mark.parametrize("pretty", [True, False]) +@pytest.mark.parametrize("remove_uids", [True, False]) +def test_write_json_pathlib(fig1, pretty, remove_uids): + # Configure file-like mock + filemock = MagicMock(spec=Path) + + # write_json to mock file + pio.write_json(fig1, filemock, pretty=pretty, remove_uids=remove_uids) + + # check write contents + expected = pio.to_json(fig1, pretty=pretty, remove_uids=remove_uids) + filemock.write_text.assert_called_once_with(expected) + + @pytest.mark.parametrize("pretty", [True, False]) @pytest.mark.parametrize("remove_uids", [True, False]) def test_write_json_from_file_string(fig1, pretty, remove_uids): diff --git a/packages/python/plotly/plotly/tests/test_optional/test_kaleido/test_kaleido.py b/packages/python/plotly/plotly/tests/test_optional/test_kaleido/test_kaleido.py index 4843ae46ffd..b6ba5ee11f8 100644 --- a/packages/python/plotly/plotly/tests/test_optional/test_kaleido/test_kaleido.py +++ b/packages/python/plotly/plotly/tests/test_optional/test_kaleido/test_kaleido.py @@ -1,16 +1,56 @@ import plotly.io as pio import plotly.io.kaleido -import sys from contextlib import contextmanager - -if sys.version_info >= (3, 3): - from unittest.mock import Mock -else: - from mock import Mock +from io import BytesIO +from pathlib import Path +from unittest.mock import Mock fig = {"layout": {"title": {"text": "figure title"}}} +def make_writeable_mocks(): + """Produce some mocks which we will use for testing the `write_image()` function. + + These mocks should be passed as the `file=` argument to `write_image()`. + + The tests should verify that the method specified in the `active_write_function` + attribute is called once, and that scope.transform is called with the `format=` + argument specified by the `.expected_format` attribute. + + In total we provide two mocks: one for a writable file descriptor, and other for a + pathlib.Path object. + """ + + # Part 1: A mock for a file descriptor + # ------------------------------------ + mock_file_descriptor = Mock() + + # A file descriptor has no write_bytes method, unlike a pathlib Path. + del mock_file_descriptor.write_bytes + + # The expected write method for a file descriptor is .write + mock_file_descriptor.active_write_function = mock_file_descriptor.write + + # Since there is no filename, there should be no format detected. + mock_file_descriptor.expected_format = None + + # Part 2: A mock for a pathlib path + # --------------------------------- + mock_pathlib_path = Mock(spec=Path) + + # A pathlib Path object has no write method, unlike a file descriptor. + del mock_pathlib_path.write + + # The expected write method for a pathlib Path is .write_bytes + mock_pathlib_path.active_write_function = mock_pathlib_path.write_bytes + + # Mock a path with PNG suffix + mock_pathlib_path.suffix = ".png" + mock_pathlib_path.expected_format = "png" + + return mock_file_descriptor, mock_pathlib_path + + @contextmanager def mocked_scope(): # Code to acquire resource, e.g.: @@ -44,15 +84,19 @@ def test_kaleido_engine_to_image(): def test_kaleido_engine_write_image(): - writeable_mock = Mock() - with mocked_scope() as scope: - pio.write_image(fig, writeable_mock, engine="kaleido", validate=False) + for writeable_mock in make_writeable_mocks(): + with mocked_scope() as scope: + pio.write_image(fig, writeable_mock, engine="kaleido", validate=False) - scope.transform.assert_called_with( - fig, format=None, width=None, height=None, scale=None - ) + scope.transform.assert_called_with( + fig, + format=writeable_mock.expected_format, + width=None, + height=None, + scale=None, + ) - assert writeable_mock.write.call_count == 1 + assert writeable_mock.active_write_function.call_count == 1 def test_kaleido_engine_to_image_kwargs(): @@ -73,24 +117,24 @@ def test_kaleido_engine_to_image_kwargs(): def test_kaleido_engine_write_image_kwargs(): - writeable_mock = Mock() - with mocked_scope() as scope: - pio.write_image( - fig, - writeable_mock, - format="jpg", - width=700, - height=600, - scale=2, - engine="kaleido", - validate=False, + for writeable_mock in make_writeable_mocks(): + with mocked_scope() as scope: + pio.write_image( + fig, + writeable_mock, + format="jpg", + width=700, + height=600, + scale=2, + engine="kaleido", + validate=False, + ) + + scope.transform.assert_called_with( + fig, format="jpg", width=700, height=600, scale=2 ) - scope.transform.assert_called_with( - fig, format="jpg", width=700, height=600, scale=2 - ) - - assert writeable_mock.write.call_count == 1 + assert writeable_mock.active_write_function.call_count == 1 def test_image_renderer(): @@ -105,3 +149,17 @@ def test_image_renderer(): height=renderer.height, scale=renderer.scale, ) + + +def test_bytesio(): + """Verify that writing to a BytesIO object contains the same data as to_image(). + + The goal of this test is to ensure that Plotly correctly handles a writable buffer + which doesn't correspond to a filesystem path. + """ + bio = BytesIO() + pio.write_image(fig, bio, format="jpg", engine="kaleido", validate=False) + bio.seek(0) # Rewind to the beginning of the buffer, otherwise read() returns b''. + bio_bytes = bio.read() + to_image_bytes = pio.to_image(fig, format="jpg", engine="kaleido", validate=False) + assert bio_bytes == to_image_bytes diff --git a/packages/python/plotly/plotly/tests/test_orca/test_to_image.py b/packages/python/plotly/plotly/tests/test_orca/test_to_image.py index a4f88b203a4..5343c29456a 100644 --- a/packages/python/plotly/plotly/tests/test_orca/test_to_image.py +++ b/packages/python/plotly/plotly/tests/test_orca/test_to_image.py @@ -6,6 +6,7 @@ import pytest import sys import pandas as pd +from io import BytesIO if sys.version_info >= (3, 3): from unittest.mock import MagicMock @@ -234,7 +235,12 @@ def test_write_image_writeable(fig1, format): fig1, mock_file, format=format, width=700, height=500, engine="orca" ) - mock_file.write.assert_called_once_with(expected_bytes) + if mock_file.write_bytes.called: + mock_file.write_bytes.assert_called_once_with(expected_bytes) + elif mock_file.write.called: + mock_file.write.assert_called_once_with(expected_bytes) + else: + assert "Neither write nor write_bytes was called." def test_write_image_string_format_inference(fig1, format): @@ -347,3 +353,17 @@ def test_invalid_figure_json(): ) assert "400: invalid or malformed request syntax" in str(err.value) + + +def test_bytesio(fig1): + """Verify that writing to a BytesIO object contains the same data as to_image(). + + The goal of this test is to ensure that Plotly correctly handles a writable buffer + which doesn't correspond to a filesystem path. + """ + bio = BytesIO() + pio.write_image(fig1, bio, format="jpg", validate=False) + bio.seek(0) + bio_bytes = bio.read() + to_image_bytes = pio.to_image(fig1, format="jpg", validate=False) + assert bio_bytes == to_image_bytes