chore(lib): use pathlib.Path instead of str (#133)

* wip(lib): change os.path to pathlib.Path

* chore(lib): use pathlib.Path instead of str

* fix(logger): convert Path to str

* chore(lint): add type hint to prevent future errors

* fix(lib): correct suffix addition

manim_slides/commons.py

@@ -1,3 +1,4 @@
+from pathlib import Path
 from typing import Any, Callable
 
 import click
@@ -18,7 +19,7 @@ def config_path_option(function: F) -> F:
         "config_path",
         metavar="FILE",
         default=CONFIG_PATH,
-        type=click.Path(dir_okay=False),
+        type=click.Path(dir_okay=False, path_type=Path),
         help="Set path to configuration file.",
         show_default=True,
     )
@@ -73,7 +74,7 @@ def folder_path_option(function: F) -> F:
         "--folder",
         metavar="DIRECTORY",
         default=FOLDER_PATH,
-        type=click.Path(exists=True, file_okay=False),
+        type=click.Path(exists=True, file_okay=False, path_type=Path),
         help="Set slides folder.",
         show_default=True,
     )

manim_slides/config.py

@@ -4,24 +4,25 @@ import shutil
 import subprocess
 import tempfile
 from enum import Enum
-from typing import Callable, Dict, List, Optional, Set, Union
+from pathlib import Path
+from typing import Dict, List, Optional, Set, Union
 
-from pydantic import BaseModel, root_validator, validator
+from pydantic import BaseModel, FilePath, root_validator, validator
 from PySide6.QtCore import Qt
 
 from .manim import FFMPEG_BIN, logger
 
 
-def merge_basenames(files: List[str]) -> str:
+def merge_basenames(files: List[FilePath]) -> Path:
     """
     Merge multiple filenames by concatenating basenames.
     """
     logger.info(f"Generating a new filename for animations: {files}")
 
-    dirname = os.path.dirname(files[0])
+    dirname = files[0].parent
-    _, ext = os.path.splitext(files[0])
+    ext = files[0].suffix
 
-    basenames = (os.path.splitext(os.path.basename(file))[0] for file in files)
+    basenames = (file.stem for file in files)
 
     basenames_str = ",".join(f"{len(b)}:{b}" for b in basenames)
 
@@ -29,7 +30,7 @@ def merge_basenames(files: List[str]) -> str:
     # https://github.com/jeertmans/manim-slides/issues/123
     basename = hashlib.sha256(basenames_str.encode()).hexdigest()
 
-    return os.path.join(dirname, basename + ext)
+    return dirname / (basename + ext)
 
 
 class Key(BaseModel):  # type: ignore
@@ -146,24 +147,12 @@ class SlideConfig(BaseModel):  # type: ignore
 
 class PresentationConfig(BaseModel):  # type: ignore
     slides: List[SlideConfig]
-    files: List[str]
+    files: List[FilePath]
-
-    @validator("files", pre=True, each_item=True)
-    def is_file_and_exists(cls, v: str) -> str:
-        if not os.path.exists(v):
-            raise ValueError(
-                f"Animation file {v} does not exist. Are you in the right directory?"
-            )
-
-        if not os.path.isfile(v):
-            raise ValueError(f"Animation file {v} is not a file")
-
-        return v
 
     @root_validator
     def animation_indices_match_files(
-        cls, values: Dict[str, Union[List[SlideConfig], List[str]]]
+        cls, values: Dict[str, Union[List[SlideConfig], List[FilePath]]]
-    ) -> Dict[str, Union[List[SlideConfig], List[str]]]:
+    ) -> Dict[str, Union[List[SlideConfig], List[FilePath]]]:
         files = values.get("files")
         slides = values.get("slides")
 
@@ -180,26 +169,21 @@ class PresentationConfig(BaseModel):  # type: ignore
 
         return values
 
-    def move_to(self, dest: str, copy: bool = True) -> "PresentationConfig":
+    def copy_to(self, dest: Path) -> "PresentationConfig":
         """
-        Moves (or copy) the files to a given directory.
+        Copy the files to a given directory.
         """
-        copy_func: Callable[[str, str], None] = shutil.copy
-        move_func: Callable[[str, str], None] = shutil.move
-        move = copy_func if copy else move_func
-
         n = len(self.files)
         for i in range(n):
             file = self.files[i]
-            basename = os.path.basename(file)
+            dest_path = dest / self.files[i].name
-            dest_path = os.path.join(dest, basename)
             logger.debug(f"Moving / copying {file} to {dest_path}")
-            move(file, dest_path)
+            shutil.copy(file, dest_path)
             self.files[i] = dest_path
 
         return self
 
-    def concat_animations(self, dest: Optional[str] = None) -> "PresentationConfig":
+    def concat_animations(self, dest: Optional[Path] = None) -> "PresentationConfig":
         """
         Concatenate animations such that each slide contains one animation.
         """
@@ -216,7 +200,7 @@ class PresentationConfig(BaseModel):  # type: ignore
                 f.writelines(f"file '{os.path.abspath(path)}'\n" for path in files)
                 f.close()
 
-                command = [
+                command: List[str] = [
                     FFMPEG_BIN,
                     "-f",
                     "concat",
@@ -226,7 +210,7 @@ class PresentationConfig(BaseModel):  # type: ignore
                     f.name,
                     "-c",
                     "copy",
-                    dest_path,
+                    str(dest_path),
                     "-y",
                 ]
                 logger.debug(" ".join(command))
@@ -252,7 +236,7 @@ class PresentationConfig(BaseModel):  # type: ignore
         self.files = dest_paths
 
         if dest:
-            return self.move_to(dest)
+            return self.copy_to(dest)
 
         return self
 

manim_slides/convert.py

@@ -1,6 +1,7 @@
 import os
 import webbrowser
 from enum import Enum
+from pathlib import Path
 from typing import Any, Callable, Dict, Generator, List, Optional, Type, Union
 
 import click
@@ -35,7 +36,7 @@ class Converter(BaseModel):  # type: ignore
     assets_dir: str = "{basename}_assets"
     template: Optional[str] = None
 
-    def convert_to(self, dest: str) -> None:
+    def convert_to(self, dest: Path) -> None:
         """Converts self, i.e., a list of presentations, into a given format."""
         raise NotImplementedError
 
@@ -45,7 +46,7 @@ class Converter(BaseModel):  # type: ignore
         An empty string is returned if no template is used."""
         return ""
 
-    def open(self, file: str) -> bool:
+    def open(self, file: Path) -> bool:
         """Opens a file, generated with converter, using appropriate application."""
         raise NotImplementedError
 
@@ -285,12 +286,12 @@ class RevealJS(Converter):
         use_enum_values = True
         extra = "forbid"
 
-    def get_sections_iter(self) -> Generator[str, None, None]:
+    def get_sections_iter(self, assets_dir: Path) -> Generator[str, None, None]:
         """Generates a sequence of sections, one per slide, that will be included into the html template."""
         for presentation_config in self.presentation_configs:
             for slide_config in presentation_config.slides:
                 file = presentation_config.files[slide_config.start_animation]
-                file = os.path.join(self.assets_dir, os.path.basename(file))
+                file = assets_dir / file.name
 
                 # TODO: document this
                 # Videos are muted because, otherwise, the first slide never plays correctly.
@@ -312,26 +313,27 @@ class RevealJS(Converter):
             __name__, "data/revealjs_template.html"
         ).decode()
 
-    def open(self, file: str) -> bool:
+    def open(self, file: Path) -> bool:
-        return webbrowser.open(file)
+        return webbrowser.open(file.absolute().as_uri())
 
-    def convert_to(self, dest: str) -> None:
+    def convert_to(self, dest: Path) -> None:
         """Converts this configuration into a RevealJS HTML presentation, saved to DEST."""
-        dirname = os.path.dirname(dest)
+        dirname = dest.parent
-        basename, ext = os.path.splitext(os.path.basename(dest))
+        basename = dest.stem
+        ext = dest.suffix
 
-        self.assets_dir = self.assets_dir.format(
+        assets_dir = Path(
-            dirname=dirname, basename=basename, ext=ext
+            self.assets_dir.format(dirname=dirname, basename=basename, ext=ext)
         )
-        full_assets_dir = os.path.join(dirname, self.assets_dir)
+        full_assets_dir = dirname / assets_dir
 
         os.makedirs(full_assets_dir, exist_ok=True)
 
         for presentation_config in self.presentation_configs:
-            presentation_config.concat_animations().move_to(full_assets_dir)
+            presentation_config.concat_animations().copy_to(full_assets_dir)
 
         with open(dest, "w") as f:
-            sections = "".join(self.get_sections_iter())
+            sections = "".join(self.get_sections_iter(full_assets_dir))
 
             revealjs_template = self.load_template()
             content = revealjs_template.format(sections=sections, **self.dict())
@@ -396,7 +398,7 @@ def show_template_option(function: Callable[..., Any]) -> Callable[..., Any]:
 @click.command()
 @click.argument("scenes", nargs=-1)
 @folder_path_option
-@click.argument("dest")
+@click.argument("dest", type=click.Path(dir_okay=False, path_type=Path))
 @click.option(
     "--to",
     type=click.Choice(["html"], case_sensitive=False),
@@ -423,7 +425,7 @@ def show_template_option(function: Callable[..., Any]) -> Callable[..., Any]:
     "--use-template",
     "template",
     metavar="FILE",
-    type=click.Path(exists=True, dir_okay=False),
+    type=click.Path(exists=True, dir_okay=False, path_type=Path),
     help="Use the template given by FILE instead of default one. To echo the default template, use `--show-template`.",
 )
 @show_template_option
@@ -431,13 +433,13 @@ def show_template_option(function: Callable[..., Any]) -> Callable[..., Any]:
 @verbosity_option
 def convert(
     scenes: List[str],
-    folder: str,
+    folder: Path,
-    dest: str,
+    dest: Path,
     to: str,
     open_result: bool,
     force: bool,
     config_options: Dict[str, str],
-    template: Optional[str],
+    template: Optional[Path],
 ) -> None:
     """
     Convert SCENE(s) into a given format and writes the result in DEST.

manim_slides/present.py

@@ -3,6 +3,7 @@ import platform
 import sys
 import time
 from enum import Enum, IntEnum, auto, unique
+from pathlib import Path
 from typing import Any, Dict, List, Optional, Tuple, Union
 
 import click
@@ -126,7 +127,7 @@ class Presentation:
 
             self.current_file = file
 
-            self.cap = cv2.VideoCapture(file)
+            self.cap = cv2.VideoCapture(str(file))
             self.loaded_animation_cap = animation
 
     @property
@@ -626,43 +627,41 @@ class App(QWidget):  # type: ignore
     "--folder",
     metavar="DIRECTORY",
     default=FOLDER_PATH,
-    type=click.Path(exists=True, file_okay=False),
+    type=click.Path(exists=True, file_okay=False, path_type=Path),
     help="Set slides folder.",
     show_default=True,
 )
 @click.help_option("-h", "--help")
 @verbosity_option
-def list_scenes(folder: str) -> None:
+def list_scenes(folder: Path) -> None:
     """List available scenes."""
 
     for i, scene in enumerate(_list_scenes(folder), start=1):
         click.secho(f"{i}: {scene}", fg="green")
 
 
-def _list_scenes(folder: str) -> List[str]:
+def _list_scenes(folder: Path) -> List[str]:
     """Lists available scenes in given directory."""
     scenes = []
 
-    for file in os.listdir(folder):
+    for filepath in folder.glob("*.json"):
-        if file.endswith(".json"):
+        try:
-            filepath = os.path.join(folder, file)
+            _ = PresentationConfig.parse_file(filepath)
-            try:
+            scenes.append(filepath.stem)
-                _ = PresentationConfig.parse_file(filepath)
+        except (
-                scenes.append(os.path.basename(file)[:-5])
+            Exception
-            except (
+        ) as e:  # Could not parse this file as a proper presentation config
-                Exception
+            logger.warn(
-            ) as e:  # Could not parse this file as a proper presentation config
+                f"Something went wrong with parsing presentation config `{filepath}`: {e}"
-                logger.warn(
+            )
-                    f"Something went wrong with parsing presentation config `{filepath}`: {e}"
+            pass
-                )
-                pass
 
     logger.debug(f"Found {len(scenes)} valid scene configuration files in `{folder}`.")
 
     return scenes
 
 
-def prompt_for_scenes(folder: str) -> List[str]:
+def prompt_for_scenes(folder: Path) -> List[str]:
     """Prompts the user to select scenes within a given folder."""
 
     scene_choices = dict(enumerate(_list_scenes(folder), start=1))
@@ -697,7 +696,7 @@ def prompt_for_scenes(folder: str) -> List[str]:
 
 
 def get_scenes_presentation_config(
-    scenes: List[str], folder: str
+    scenes: List[str], folder: Path
 ) -> List[PresentationConfig]:
     """Returns a list of presentation configurations based on the user input."""
 
@@ -706,8 +705,8 @@ def get_scenes_presentation_config(
 
     presentation_configs = []
     for scene in scenes:
-        config_file = os.path.join(folder, f"{scene}.json")
+        config_file = folder / f"{scene}.json"
-        if not os.path.exists(config_file):
+        if not config_file.exists():
             raise click.UsageError(
                 f"File {config_file} does not exist, check the scene name and make sure to use Slide as your scene base class"
             )
@@ -726,7 +725,7 @@ def get_scenes_presentation_config(
     "--folder",
     metavar="DIRECTORY",
     default=FOLDER_PATH,
-    type=click.Path(exists=True, file_okay=False),
+    type=click.Path(exists=True, file_okay=False, path_type=Path),
     help="Set slides folder.",
     show_default=True,
 )
@@ -752,7 +751,7 @@ def get_scenes_presentation_config(
     "--record-to",
     "record_to",
     metavar="FILE",
-    type=click.Path(dir_okay=False),
+    type=click.Path(dir_okay=False, path_type=Path),
     default=None,
     help="If set, the presentation will be recorded into a AVI video file with given name.",
 )
@@ -794,13 +793,13 @@ def get_scenes_presentation_config(
 @verbosity_option
 def present(
     scenes: List[str],
-    config_path: str,
+    config_path: Path,
-    folder: str,
+    folder: Path,
     start_paused: bool,
     fullscreen: bool,
     skip_all: bool,
     resolution: Tuple[int, int],
-    record_to: Optional[str],
+    record_to: Optional[Path],
     exit_after_last_slide: bool,
     hide_mouse: bool,
     aspect_ratio: str,
@@ -825,7 +824,7 @@ def present(
         for presentation_config in get_scenes_presentation_config(scenes, folder)
     ]
 
-    if os.path.exists(config_path):
+    if config_path.exists():
         try:
             config = Config.parse_file(config_path)
         except ValidationError as e:
@@ -835,7 +834,7 @@ def present(
         config = Config()
 
     if record_to is not None:
-        _, ext = os.path.splitext(record_to)
+        ext = record_to.suffix
         if ext.lower() != ".avi":
             raise click.UsageError(
                 "Recording only support '.avi' extension. For other video formats, please convert the resulting '.avi' file afterwards."