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

Author: jeertmans

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,32 +4,33 @@
 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])
-    _, ext = os.path.splitext(files[0])
+    dirname = files[0].parent
+    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)
 
     # We use hashes to prevent too-long filenames, see issue #123:
     # 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 @@ def slides_slice(self) -> slice:
 
 class PresentationConfig(BaseModel):  # type: ignore
     slides: List[SlideConfig]
-    files: List[str]
-
-    @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
+    files: List[FilePath]
 
     @root_validator
     def animation_indices_match_files(
-        cls, values: Dict[str, Union[List[SlideConfig], List[str]]]
-    ) -> Dict[str, Union[List[SlideConfig], List[str]]]:
+        cls, values: Dict[str, Union[List[SlideConfig], List[FilePath]]]
+    ) -> Dict[str, Union[List[SlideConfig], List[FilePath]]]:
         files = values.get("files")
         slides = values.get("slides")
 
@@ -180,26 +169,21 @@ def animation_indices_match_files(
 
         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 = os.path.join(dest, basename)
+            dest_path = dest / self.files[i].name
             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 @@ def concat_animations(self, dest: Optional[str] = None) -> "PresentationConfig":
                 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 @@ def concat_animations(self, dest: Optional[str] = None) -> "PresentationConfig":
                     f.name,
                     "-c",
                     "copy",
-                    dest_path,
+                    str(dest_path),
                     "-y",
                 ]
                 logger.debug(" ".join(command))
@@ -252,7 +236,7 @@ def concat_animations(self, dest: Optional[str] = None) -> "PresentationConfig":
         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 @@ def load_template(self) -> str:
         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 Config:
         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 @@ def load_template(self) -> str:
             __name__, "data/revealjs_template.html"
         ).decode()
 
-    def open(self, file: str) -> bool:
-        return webbrowser.open(file)
+    def open(self, file: Path) -> bool:
+        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)
-        basename, ext = os.path.splitext(os.path.basename(dest))
+        dirname = dest.parent
+        basename = dest.stem
+        ext = dest.suffix
 
-        self.assets_dir = self.assets_dir.format(
-            dirname=dirname, basename=basename, ext=ext
+        assets_dir = Path(
+            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 callback(ctx: Context, param: Parameter, value: bool) -> None:
 @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,21 +425,21 @@ def callback(ctx: Context, param: Parameter, value: bool) -> None:
     "--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
 @show_config_options
 @verbosity_option
 def convert(
     scenes: List[str],
-    folder: str,
-    dest: str,
+    folder: Path,
+    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.