Improve code/test comments.

Author: maresb

packages/python/plotly/plotly/io/_kaleido.py

@@ -2,8 +2,6 @@
 from six import string_types
 import os
 import sys
-
-
 import json
 import plotly
 from plotly.io._utils import validate_coerce_fig_to_dict
@@ -224,13 +222,17 @@ def write_image(
     -------
     None
     """
-    # Check if file can be interpreted as a pathlib object
-    # ----------------------------------------------------
+    # Try to cast `file` as a pathlib object `path`.
+    # ----------------------------------------------
     if isinstance(file, string_types):
+        # Use the standard pathlib constructor to make a pathlib object.
         path = Path(file)
-    elif isinstance(file, PurePath):
+    elif isinstance(file, PurePath):  # 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
 
     # Infer format if not specified
@@ -268,11 +270,9 @@ def write_image(
 
     # Open file
     # ---------
-    if path is not None:
-        path.write_bytes(img_data)
-    else:
-        # Handle an open file descriptor
-        # ------------------------------
+    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
@@ -285,6 +285,10 @@ def write_image(
                 file=file
             )
         )
+    else:
+        # 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):

packages/python/plotly/plotly/tests/test_optional/test_kaleido/test_kaleido.py

@@ -19,9 +19,20 @@
 
 
 def make_writeable_mocks():
+    """Produce some mocks which we will use for testing the `write_image()` function.
 
-    # A mock for a file descriptor
-    # ----------------------------
+    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.
@@ -33,8 +44,8 @@ def make_writeable_mocks():
     # Since there is no filename, there should be no format detected.
     mock_file_descriptor.expected_format = None
 
-    # A mock for a pathlib path
-    # -------------------------
+    # 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.
@@ -151,10 +162,14 @@ def test_image_renderer():
 
 
 def test_bytesio():
-    # Verify that writing to a BytesIO object contains the same data as with to_image().
+    """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)
+    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

packages/python/plotly/plotly/tests/test_orca/test_to_image.py

@@ -343,7 +343,11 @@ def test_invalid_figure_json():
 
 
 def test_bytesio(fig1):
-    # Verify that writing to a BytesIO object contains the same data as with to_image().
+    """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)