Skip to content

Commit d0b8077

Browse files
committed
Improve code/test comments.
1 parent 7d4176b commit d0b8077

File tree

3 files changed

+40
-17
lines changed

3 files changed

+40
-17
lines changed

Diff for: packages/python/plotly/plotly/io/_kaleido.py

+14-10
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
from six import string_types
33
import os
44
import sys
5-
6-
75
import json
86
import plotly
97
from plotly.io._utils import validate_coerce_fig_to_dict
@@ -224,13 +222,17 @@ def write_image(
224222
-------
225223
None
226224
"""
227-
# Check if file can be interpreted as a pathlib object
228-
# ----------------------------------------------------
225+
# Try to cast `file` as a pathlib object `path`.
226+
# ----------------------------------------------
229227
if isinstance(file, string_types):
228+
# Use the standard pathlib constructor to make a pathlib object.
230229
path = Path(file)
231-
elif isinstance(file, PurePath):
230+
elif isinstance(file, PurePath): # PurePath is the most general pathlib object.
231+
# `file` is already a pathlib object.
232232
path = file
233233
else:
234+
# We could not make a pathlib object out of file. Either `file` is an open file
235+
# descriptor with a `write()` method or it's an invalid object.
234236
path = None
235237

236238
# Infer format if not specified
@@ -268,11 +270,9 @@ def write_image(
268270

269271
# Open file
270272
# ---------
271-
if path is not None:
272-
path.write_bytes(img_data)
273-
else:
274-
# Handle an open file descriptor
275-
# ------------------------------
273+
if path is None:
274+
# We previously failed to make sense of `file` as a pathlib object.
275+
# Attempt to write to `file` as an open file descriptor.
276276
try:
277277
file.write(img_data)
278278
return
@@ -285,6 +285,10 @@ def write_image(
285285
file=file
286286
)
287287
)
288+
else:
289+
# We previously succeeded in interpreting `file` as a pathlib object.
290+
# Now we can use `write_bytes()`.
291+
path.write_bytes(img_data)
288292

289293

290294
def full_figure_for_development(fig, warn=True, as_dict=False):

Diff for: packages/python/plotly/plotly/tests/test_optional/test_kaleido/test_kaleido.py

+21-6
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,20 @@
1919

2020

2121
def make_writeable_mocks():
22+
"""Produce some mocks which we will use for testing the `write_image()` function.
2223
23-
# A mock for a file descriptor
24-
# ----------------------------
24+
These mocks should be passed as the `file=` argument to `write_image()`.
25+
26+
The tests should verify that the method specified in the `active_write_function`
27+
attribute is called once, and that scope.transform is called with the `format=`
28+
argument specified by the `.expected_format` attribute.
29+
30+
In total we provide two mocks: one for a writable file descriptor, and other for a
31+
pathlib.Path object.
32+
"""
33+
34+
# Part 1: A mock for a file descriptor
35+
# ------------------------------------
2536
mock_file_descriptor = Mock()
2637

2738
# A file descriptor has no write_bytes method, unlike a pathlib Path.
@@ -33,8 +44,8 @@ def make_writeable_mocks():
3344
# Since there is no filename, there should be no format detected.
3445
mock_file_descriptor.expected_format = None
3546

36-
# A mock for a pathlib path
37-
# -------------------------
47+
# Part 2: A mock for a pathlib path
48+
# ---------------------------------
3849
mock_pathlib_path = Mock(spec=Path)
3950

4051
# A pathlib Path object has no write method, unlike a file descriptor.
@@ -151,10 +162,14 @@ def test_image_renderer():
151162

152163

153164
def test_bytesio():
154-
# Verify that writing to a BytesIO object contains the same data as with to_image().
165+
"""Verify that writing to a BytesIO object contains the same data as to_image().
166+
167+
The goal of this test is to ensure that Plotly correctly handles a writable buffer
168+
which doesn't correspond to a filesystem path.
169+
"""
155170
bio = BytesIO()
156171
pio.write_image(fig, bio, format="jpg", engine="kaleido", validate=False)
157-
bio.seek(0)
172+
bio.seek(0) # Rewind to the beginning of the buffer, otherwise read() returns b''.
158173
bio_bytes = bio.read()
159174
to_image_bytes = pio.to_image(fig, format="jpg", engine="kaleido", validate=False)
160175
assert bio_bytes == to_image_bytes

Diff for: packages/python/plotly/plotly/tests/test_orca/test_to_image.py

+5-1
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,11 @@ def test_invalid_figure_json():
343343

344344

345345
def test_bytesio(fig1):
346-
# Verify that writing to a BytesIO object contains the same data as with to_image().
346+
"""Verify that writing to a BytesIO object contains the same data as to_image().
347+
348+
The goal of this test is to ensure that Plotly correctly handles a writable buffer
349+
which doesn't correspond to a filesystem path.
350+
"""
347351
bio = BytesIO()
348352
pio.write_image(fig1, bio, format="jpg", validate=False)
349353
bio.seek(0)

0 commit comments

Comments
 (0)