save_file now cleans the filename, expands user and returns the full file path (#84)

Author: Nabil Freij

.pre-commit-config.yaml

@@ -5,13 +5,13 @@ repos:
       - id: docformatter
         args: [--in-place, --pre-summary-newline, --make-summary-multi]
   - repo: https://github.com/myint/autoflake
-    rev: v1.5.3
+    rev: v1.7.6
     hooks:
       - id: autoflake
         args: ['--in-place', '--remove-all-unused-imports', '--remove-unused-variable']
         exclude: ".*(.fits|.fts|.fit|.txt|tca.*|extern.*|.rst|.md|__init__.py)$"
   - repo: https://github.com/psf/black
-    rev: 22.8.0
+    rev: 22.10.0
     hooks:
     - id: black
       exclude: ".*(.fits|.fts|.fit|.txt|.csv)$"
@@ -34,7 +34,7 @@ repos:
     - id: check-yaml
     - id: debug-statements
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: 'v0.971'
+    rev: 'v0.982'
     hooks:
     - id: mypy
       additional_dependencies: [types-requests==2.28.0]

CHANGELOG.rst

@@ -30,3 +30,8 @@ You need to install the development version of the package in order to make and
 See the :ref:`obtaining_the_source` section for details.
 
 You may also want to familiarize yourself with the :ref:`dev_guide` for ``hvpy``.
+
+Changelog
+---------
+
+The changelog for ``hvpy`` is available on the `GitHub releases page <https://github.com/Helioviewer-Project/python-api/releases>`__.

docs/index.rst

@@ -122,12 +122,12 @@ def createMovie(
         filename = f"{title}.{format}"
     else:
         filename = f"{filename}.{format}"
-    save_file(
+    filename = save_file(
         data=binary_data,
         filename=filename,
         overwrite=overwrite,
     )
-    return Path(filename)
+    return filename
 
 
 @_add_shared_docstring(takeScreenshotInputParameters)
@@ -203,9 +203,9 @@ def createScreenshot(
         filename = f"{res['id']}_{date.date()}.png"
     else:
         filename = f"{filename}.png"
-    save_file(
+    filename = save_file(
         data=binary_data,
         filename=filename,
         overwrite=overwrite,
     )
-    return Path(filename)
+    return filename

hvpy/helpers.py

@@ -38,7 +38,7 @@ def test_createMovie(start_time, end_time, tmp_path):
     assert result == tmp_path / "movie.mp4"
 
 
-def test_createMovie_with_none_filename(start_time, end_time):
+def test_createMovie_with_no_filename(start_time, end_time):
     result = createMovie(
         startTime=start_time,
         endTime=end_time,

hvpy/tests/test_helper.py

@@ -1,3 +1,4 @@
+from pathlib import Path
 from datetime import datetime
 
 import pytest
@@ -71,7 +72,7 @@ def test_create_events_string():
 
 
 def test_save_file(tmp_path):
-    f1 = tmp_path / "test.png"
+    filename = tmp_path / "test.png"
     res = takeScreenshot(
         date=datetime.today(),
         imageScale=2.44,
@@ -82,13 +83,47 @@ def test_save_file(tmp_path):
         height=1200,
         display=True,
     )
-    save_file(res, f1, overwrite=False)
-    assert f1.exists()
+    saved_file = save_file(res, filename, overwrite=False)
+    assert saved_file == filename
     with pytest.raises(FileExistsError, match="already exists"):
-        save_file(res, f1, overwrite=False)
-    save_file(res, f1, overwrite=True)
-    assert f1.exists()
+        save_file(res, filename, overwrite=False)
+    save_file(res, filename, overwrite=True)
 
-    f2 = tmp_path / "test2.png"
-    save_file(res, str(f2), overwrite=False)
-    assert f2.exists()
+
+def test_save_file_cleans(tmp_path):
+    # Clean the filename for Windows filepaths
+    filename = tmp_path / ":test.png"
+    clean_filename = str(filename).replace(":test.png", "_test.png")
+    res = takeScreenshot(
+        date=datetime.today(),
+        imageScale=2.44,
+        layers="[10,1,100]",
+        x0=0,
+        y0=0,
+        width=1920,
+        height=1200,
+        display=True,
+    )
+    saved_file = save_file(res, str(filename))
+    assert not filename.exists()
+    assert saved_file == Path(clean_filename)
+
+
+def test_save_file_expands():
+    # Check that ~/ expands
+    filename = "~/:test.png"
+    clean_filename = str(filename).replace(":", "_")
+    res = takeScreenshot(
+        date=datetime.today(),
+        imageScale=2.44,
+        layers="[10,1,100]",
+        x0=0,
+        y0=0,
+        width=1920,
+        height=1200,
+        display=True,
+    )
+    saved_file = save_file(res, filename)
+    saved_file.unlink()
+    assert not Path(filename).exists()
+    assert saved_file == Path(clean_filename).expanduser().resolve()

hvpy/tests/test_utils.py

@@ -1,3 +1,5 @@
+import os
+import re
 from typing import Any, List, Union, Callable
 from pathlib import Path
 from datetime import datetime
@@ -151,7 +153,7 @@ def create_events(events: List[Union[EventType, str, tuple]]) -> str:
     return constructed_events[:-1]
 
 
-def save_file(data: bytearray, filename: Union[Path, str], overwrite: bool = False) -> None:
+def save_file(data: bytearray, filename: Union[Path, str], overwrite: bool = False) -> Path:
     """
     Saves a file to the specified path.
 
@@ -164,9 +166,18 @@ def save_file(data: bytearray, filename: Union[Path, str], overwrite: bool = Fal
     overwrite
         Whether to overwrite the file if it already exists.
         Default is `False`.
+
+    Returns
+    -------
+    `~pathlib.Path`
+        The path to the saved file.
     """
-    if isinstance(filename, str):
-        filename = Path(filename)
+    filepath, filename = os.path.split(filename)
+    filename = re.sub(r"[^\w\-_\. ]", "_", filename)
+    filename = Path(filepath) / Path(filename)
+    filename = Path(filename).expanduser().resolve().absolute()
+    # Sanitize the filename - Only works for strings
     if filename.exists() and not overwrite:
         raise FileExistsError(f"{filename} already exists. Use overwrite=True to overwrite.")
     filename.write_bytes(data)
+    return filename