fix(lib/cli): relative paths, empty slides, and tests (#223)

* fix(lib/cli): relative paths, empty slides, and tests

This fixes two issues:

1. Empty slides are now reported as error, to prevent indexing error;
2. Changing the folder path will now produce an absolute path to slides, which was not the case before and would lead to a "file does not exist error".

A few tests were also added to cover those

* fix(lib): fix from_file, remove useless field, and more

* chore(tests): remove print

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

.gitignore

@@ -10,6 +10,7 @@ __pycache__/
 /.vscode
 
 slides/
+!tests/slides/
 
 .manim-slides.json
 

manim_slides/config.py

@@ -1,4 +1,5 @@
 import hashlib
+import json
 import os
 import shutil
 import subprocess
@@ -7,7 +8,14 @@ from enum import Enum
 from pathlib import Path
 from typing import Any, Dict, List, Optional, Set, Tuple, Union
 
-from pydantic import BaseModel, FilePath, PositiveInt, field_validator, model_validator
+from pydantic import (
+    BaseModel,
+    Field,
+    FilePath,
+    PositiveInt,
+    field_validator,
+    model_validator,
+)
 from pydantic_extra_types.color import Color
 from PySide6.QtCore import Qt
 
@@ -71,6 +79,17 @@ class Config(BaseModel):  # type: ignore
     PLAY_PAUSE: Key = Key(ids=[Qt.Key_Space], name="PLAY / PAUSE")
     HIDE_MOUSE: Key = Key(ids=[Qt.Key_H], name="HIDE / SHOW MOUSE")
 
+    @classmethod
+    def from_file(cls, path: Path) -> "Config":
+        """Reads a configuration from a file."""
+        with open(path, "r") as f:
+            return cls.model_validate_json(f.read())  # type: ignore
+
+    def to_file(self, path: Path) -> None:
+        """Dumps the configuration to a file."""
+        with open(path, "w") as f:
+            f.write(self.model_dump_json(indent=2))
+
     @model_validator(mode="before")
     def ids_are_unique_across_keys(cls, values: Dict[str, Key]) -> Dict[str, Key]:
         ids: Set[int] = set()
@@ -104,7 +123,7 @@ class SlideConfig(BaseModel):  # type: ignore
     start_animation: int
     end_animation: int
     number: int
-    terminated: bool = False
+    terminated: bool = Field(False, exclude=True)
 
     @field_validator("start_animation", "end_animation")
     @classmethod
@@ -151,11 +170,31 @@ class SlideConfig(BaseModel):  # type: ignore
 
 
 class PresentationConfig(BaseModel):  # type: ignore
-    slides: List[SlideConfig]
+    slides: List[SlideConfig] = Field(min_length=1)
     files: List[FilePath]
     resolution: Tuple[PositiveInt, PositiveInt] = (1920, 1080)
     background_color: Color = "black"
 
+    @classmethod
+    def from_file(cls, path: Path) -> "PresentationConfig":
+        """Reads a presentation configuration from a file."""
+        with open(path, "r") as f:
+            obj = json.load(f)
+
+            if files := obj.get("files", None):
+                # First parent is ../slides
+                # so we take the parent of this parent
+                parent = Path(path).parents[1]
+                for i in range(len(files)):
+                    files[i] = parent / files[i]
+
+            return cls.model_validate(obj)  # type: ignore
+
+    def to_file(self, path: Path) -> None:
+        """Dumps the presentation configuration to a file."""
+        with open(path, "w") as f:
+            f.write(self.model_dump_json(indent=2))
+
     @model_validator(mode="after")
     def animation_indices_match_files(
         cls, config: "PresentationConfig"

manim_slides/present.py

@@ -786,7 +786,7 @@ def _list_scenes(folder: Path) -> List[str]:
 
     for filepath in folder.glob("*.json"):
         try:
-            _ = PresentationConfig.parse_file(filepath)
+            _ = PresentationConfig.from_file(filepath)
             scenes.append(filepath.stem)
         except (
             Exception
@@ -851,7 +851,7 @@ def get_scenes_presentation_config(
                 f"File {config_file} does not exist, check the scene name and make sure to use Slide as your scene base class"
             )
         try:
-            presentation_configs.append(PresentationConfig.parse_file(config_file))
+            presentation_configs.append(PresentationConfig.from_file(config_file))
         except ValidationError as e:
             raise click.UsageError(str(e))
 
@@ -1047,7 +1047,7 @@ def present(
 
     if config_path.exists():
         try:
-            config = Config.parse_file(config_path)
+            config = Config.from_file(config_path)
         except ValidationError as e:
             raise click.UsageError(str(e))
     else:
@@ -1070,7 +1070,11 @@ def present(
     if start_at[2]:
         start_at_animation_number = start_at[2]
 
-    app = QApplication(sys.argv)
+    if not QApplication.instance():
+        app = QApplication(sys.argv)
+    else:
+        app = QApplication.instance()
+
     app.setApplicationName("Manim Slides")
     a = App(
         presentations,

manim_slides/wizard.py

@@ -1,6 +1,6 @@
-import os
 import sys
 from functools import partial
+from pathlib import Path
 from typing import Any
 
 import click
@@ -102,7 +102,7 @@ class Wizard(QWidget):  # type: ignore
 
     def saveConfig(self) -> None:
         try:
-            Config.parse_obj(self.config.dict())
+            Config.model_validate(self.config.dict())
         except ValueError:
             msg = QMessageBox()
             msg.setIcon(QMessageBox.Critical)
@@ -130,7 +130,7 @@ class Wizard(QWidget):  # type: ignore
 @config_options
 @click.help_option("-h", "--help")
 @verbosity_option
-def wizard(config_path: str, force: bool, merge: bool) -> None:
+def wizard(config_path: Path, force: bool, merge: bool) -> None:
     """Launch configuration wizard."""
     return _init(config_path, force, merge, skip_interactive=False)
 
@@ -140,18 +140,18 @@ def wizard(config_path: str, force: bool, merge: bool) -> None:
 @click.help_option("-h", "--help")
 @verbosity_option
 def init(
-    config_path: str, force: bool, merge: bool, skip_interactive: bool = False
+    config_path: Path, force: bool, merge: bool, skip_interactive: bool = False
 ) -> None:
     """Initialize a new default configuration file."""
     return _init(config_path, force, merge, skip_interactive=True)
 
 
 def _init(
-    config_path: str, force: bool, merge: bool, skip_interactive: bool = False
+    config_path: Path, force: bool, merge: bool, skip_interactive: bool = False
 ) -> None:
     """Actual initialization code for configuration file, with optional interactive mode."""
 
-    if os.path.exists(config_path):
+    if config_path.exists():
         click.secho(f"The `{CONFIG_PATH}` configuration file exists")
 
         if not force and not merge:
@@ -175,8 +175,8 @@ def _init(
         logger.debug("Merging new config into `{config_path}`")
 
     if not skip_interactive:
-        if os.path.exists(config_path):
+        if config_path.exists():
-            config = Config.parse_file(config_path)
+            config = Config.from_file(config_path)
 
         app = QApplication(sys.argv)
         app.setApplicationName("Manim Slides Wizard")
@@ -187,9 +187,8 @@ def _init(
         config = window.config
 
     if merge:
-        config = Config.parse_file(config_path).merge_with(config)
+        config = Config.from_file(config_path).merge_with(config)
 
-    with open(config_path, "w") as config_file:
+    config.to_file(config_path)
-        config_file.write(config.json(indent=2))
 
     click.secho(f"Configuration file successfully saved to `{config_path}`")

tests/conftest.py

@@ -1,3 +1,13 @@
+from pathlib import Path
+from typing import Iterator
+
+import pytest
+
 from manim_slides.logger import make_logger
 
 _ = make_logger()  # This is run so that "PERF" level is created
+
+
+@pytest.fixture
+def folder_path() -> Iterator[Path]:
+    yield (Path(__file__).parent / "slides").resolve()

tests/slides/BasicExample.json

@@ -0,0 +1,32 @@
+{
+  "slides": [
+    {
+      "type": "slide",
+      "start_animation": 0,
+      "end_animation": 1,
+      "number": 1
+    },
+    {
+      "type": "loop",
+      "start_animation": 1,
+      "end_animation": 2,
+      "number": 2
+    },
+    {
+      "type": "last",
+      "start_animation": 2,
+      "end_animation": 3,
+      "number": 3
+    }
+  ],
+  "files": [
+    "slides/files/BasicExample/1413466013_3346521118_223132457.mp4",
+    "slides/files/BasicExample/1672018281_3136302242_2191168284.mp4",
+    "slides/files/BasicExample/1672018281_1369283980_3942561600.mp4"
+  ],
+  "resolution": [
+    1920,
+    1080
+  ],
+  "background_color": "black"
+}

tests/test_config.py

@@ -97,3 +97,7 @@ class TestPresentationConfig:
 
     def test_bump_to_json(self, presentation_config: PresentationConfig) -> None:
         _ = presentation_config.model_dump_json(indent=2)
+
+    def test_empty_presentation_config(self) -> None:
+        with pytest.raises(ValidationError):
+            _ = PresentationConfig(slides=[], files=[])

tests/test_main.py

@@ -0,0 +1,93 @@
+from pathlib import Path
+
+from click.testing import CliRunner
+
+from manim_slides.__main__ import cli
+
+
+def test_help() -> None:
+    runner = CliRunner()
+    results = runner.invoke(cli, ["-S", "--help"])
+
+    assert results.exit_code == 0
+
+    results = runner.invoke(cli, ["-S", "-h"])
+
+    assert results.exit_code == 0
+
+
+def test_defaults_to_present(folder_path: Path) -> None:
+    runner = CliRunner()
+
+    with runner.isolated_filesystem():
+        results = runner.invoke(
+            cli, ["BasicExample", "--folder", str(folder_path), "-s"]
+        )
+
+        assert results.exit_code == 0
+
+
+def test_present(folder_path: Path) -> None:
+    runner = CliRunner()
+
+    with runner.isolated_filesystem():
+        results = runner.invoke(
+            cli, ["present", "BasicExample", "--folder", str(folder_path), "-s"]
+        )
+
+        assert results.exit_code == 0
+
+
+def test_convert(folder_path: Path) -> None:
+    runner = CliRunner()
+
+    with runner.isolated_filesystem():
+        results = runner.invoke(
+            cli,
+            [
+                "convert",
+                "BasicExample",
+                "basic_example.html",
+                "--folder",
+                str(folder_path),
+            ],
+        )
+
+        assert results.exit_code == 0
+
+
+def test_init() -> None:
+    runner = CliRunner()
+
+    with runner.isolated_filesystem():
+        results = runner.invoke(
+            cli,
+            [
+                "init",
+                "--force",
+            ],
+        )
+
+        assert results.exit_code == 0
+
+
+def test_list_scenes(folder_path: Path) -> None:
+    runner = CliRunner()
+
+    with runner.isolated_filesystem():
+        results = runner.invoke(
+            cli,
+            [
+                "list-scenes",
+                "--folder",
+                str(folder_path),
+            ],
+        )
+
+        assert results.exit_code == 0
+        assert "BasicExample" in results.output
+
+
+def test_wizard() -> None:
+    # TODO
+    pass