From 3b9d1ef1a84867a44c2e5d79a8b8ebd1778d7257 Mon Sep 17 00:00:00 2001 From: Lars Bogner Date: Wed, 22 Jul 2026 15:29:55 +0200 Subject: [PATCH] Rewrite CI to lint/typecheck/audit/test only; add HPC deployment path - Replace the Docker build/publish CI stages with ruff (lint + format check), ty (type check), pip-audit, and pytest run directly against python:3.11-slim; Docker remains for manual/server deployment only. - Swap black/pylint/mypy for ruff/ty across pyproject.toml, and fix every resulting lint, format, and type diagnostic in gallery/ and plotstyle/. - Fix tests broken/stale from before the package restructuring: wrong `utils.*` import paths, mock patch targets pointed at the wrong module, and PDF-conversion tests still assuming ImageMagick instead of the current PyMuPDF-first path. Drop test_container.py (obsolete Docker-container smoke tests, fully superseded elsewhere). - Add a plain-venv + systemd --user timer deployment path (deploy/systemd/) for HPC login nodes without a Docker daemon, where public_html is already served by existing infrastructure. - Document both in CLAUDE.md, including running CI's checks locally before committing. Co-Authored-By: Claude Sonnet 5 --- .gitlab-ci.yml | 120 +++------ CLAUDE.md | 15 +- config.lbogner.yaml | 37 +++ deploy/systemd/README.md | 62 +++++ deploy/systemd/gallery-generate.service | 7 + deploy/systemd/gallery-generate.timer | 10 + gallery/__init__.py | 37 ++- gallery/api.py | 84 ++----- gallery/builder.py | 30 +-- gallery/cli.py | 65 +++-- gallery/config/__init__.py | 28 ++- gallery/tui.py | 33 +-- gallery/utils/backup.py | 7 +- gallery/utils/metadata.py | 39 ++- gallery/utils/processing.py | 77 +++--- gallery/utils/stats.py | 4 +- plotstyle/annotations.py | 8 +- plotstyle/figures.py | 10 +- plotstyle/style.py | 4 +- pyproject.toml | 19 +- tests/__init__.py | 1 - tests/test_backup.py | 229 ++++++----------- tests/test_config.py | 318 ++++++++++++------------ tests/test_container.py | 138 ---------- tests/test_generate_gallery.py | 109 ++++---- tests/test_metadata.py | 101 ++++---- tests/validate_metadata.py | 75 +++--- 27 files changed, 752 insertions(+), 915 deletions(-) create mode 100644 config.lbogner.yaml create mode 100644 deploy/systemd/README.md create mode 100644 deploy/systemd/gallery-generate.service create mode 100644 deploy/systemd/gallery-generate.timer delete mode 100644 tests/test_container.py diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 76970ed..6da6b9f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,99 +1,61 @@ # GitLab CI/CD Pipeline for ETPlot -# Builds a Docker image and runs tests inside it. +# Lints, type-checks, and tests the package. No container build/publish. stages: - - build + - check - test - - publish + +image: python:3.11-slim variables: - IMAGE_TAG: $CI_REGISTRY_IMAGE:$CI_COMMIT_SHORT_SHA - IMAGE_LATEST: $CI_REGISTRY_IMAGE:latest + PIP_CACHE_DIR: "$CI_PROJECT_DIR/.cache/pip" -# --------------------------------------------------------------------------- -# Build -# --------------------------------------------------------------------------- +cache: + key: "$CI_COMMIT_REF_SLUG" + paths: + - .cache/pip -build:image: - stage: build - image: docker:27 - services: - - docker:27-dind - variables: - DOCKER_TLS_CERTDIR: "/certs" +.install: &install before_script: - - docker login -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD" "$CI_REGISTRY" + - pip install -e ".[dev,plotting]" + +# --------------------------------------------------------------------------- +# Check: lint, format, type check, vulnerabilities +# --------------------------------------------------------------------------- + +lint:ruff: + stage: check + <<: *install script: - - docker build --pull -t "$IMAGE_TAG" . - - docker push "$IMAGE_TAG" - rules: - - if: $CI_COMMIT_BRANCH + - ruff check gallery plotstyle tests + +format:ruff: + stage: check + <<: *install + script: + - ruff format --check gallery plotstyle tests + +typecheck:ty: + stage: check + <<: *install + script: + - ty check gallery plotstyle + +vulnerabilities:pip-audit: + stage: check + <<: *install + script: + - pip-audit # --------------------------------------------------------------------------- # Test # --------------------------------------------------------------------------- -.test_base: - stage: test - image: docker:27 - services: - - docker:27-dind - variables: - DOCKER_TLS_CERTDIR: "/certs" - before_script: - - docker login -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD" "$CI_REGISTRY" - - docker pull "$IMAGE_TAG" - needs: - - build:image - test:pytest: - extends: .test_base + stage: test + <<: *install script: - - docker run --rm -w /app "$IMAGE_TAG" python -m pytest tests/ -v + - python -m pytest tests/ -v artifacts: when: always expire_in: 30 days - -test:coverage: - extends: .test_base - script: - - | - docker run --rm -w /app \ - -v "$CI_PROJECT_DIR:/artifacts" \ - "$IMAGE_TAG" \ - python -m pytest tests/ \ - --cov=gallery \ - --cov-report=xml:/artifacts/coverage.xml \ - --cov-report=term - coverage: '/TOTAL.*\s+(\d+%)$/' - artifacts: - reports: - coverage_report: - coverage_format: cobertura - path: coverage.xml - paths: - - coverage.xml - expire_in: 30 days - -# --------------------------------------------------------------------------- -# Publish latest tag on main -# --------------------------------------------------------------------------- - -publish:latest: - stage: publish - image: docker:27 - services: - - docker:27-dind - variables: - DOCKER_TLS_CERTDIR: "/certs" - before_script: - - docker login -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD" "$CI_REGISTRY" - script: - - docker pull "$IMAGE_TAG" - - docker tag "$IMAGE_TAG" "$IMAGE_LATEST" - - docker push "$IMAGE_LATEST" - needs: - - test:pytest - - test:coverage - rules: - - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH diff --git a/CLAUDE.md b/CLAUDE.md index d75b407..e163131 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,7 +46,16 @@ gallery tui python -m http.server 8000 -d /web/kschmidt/public_html/ ``` -Code style: black with `line-length = 120`. +Code style: ruff (lint + format), `line-length = 120`. Type-checked with `ty`. + +```bash +ruff check gallery plotstyle tests +ruff format gallery plotstyle tests +ty check gallery plotstyle +pip-audit +``` + +**Before committing**, run the same checks CI (`.gitlab-ci.yml`) runs and make sure they pass — `ruff check`, `ruff format --check`, `ty check`, `pip-audit`, and `pytest tests/`. Catching a failure locally is faster than waiting on the pipeline. ## Architecture @@ -124,4 +133,6 @@ The frontend is vanilla ES modules — no build step. `assets/js/main.js` import ### Deployment -The project ships a `Dockerfile` plus `docker-compose.yml` (a `generator` service that runs `gallery generate` on an interval, and an `nginx`-based `web` service serving the output — see `deploy/entrypoint.sh` and `deploy/nginx.conf`). CI (`.gitlab-ci.yml`) builds the Docker image and runs pytest inside it. For local development the `.venv` (or `uv`) is sufficient. +The project ships a `Dockerfile` plus `docker-compose.yml` (a `generator` service that runs `gallery generate` on an interval, and an `nginx`-based `web` service serving the output — see `deploy/entrypoint.sh` and `deploy/nginx.conf`). This suits a dedicated VM/server you fully control. CI (`.gitlab-ci.yml`) runs `ruff check`, `ruff format --check`, `ty check`, `pip-audit`, and pytest directly against a `python:3.11-slim` image — no container build/publish in CI. For local development the `.venv` (or `uv`) is sufficient. + +On shared HPC login nodes without a Docker daemon (e.g. KIT ETP, where `public_html` is already auto-served) use a plain venv install plus a `systemd --user` timer instead — see `deploy/systemd/README.md`. diff --git a/config.lbogner.yaml b/config.lbogner.yaml new file mode 100644 index 0000000..ca41126 --- /dev/null +++ b/config.lbogner.yaml @@ -0,0 +1,37 @@ +# Gallery Configuration for lbogner (KIT ETP HPC login nodes) +# ============================================================== +# Deployed via plain venv + systemd --user timer (see deploy/systemd/README.md), +# not Docker/Compose — this path has no Docker daemon, and /web/lbogner/public_html +# is already served by KIT's own web infrastructure. + +paths: + # Working directory where the script runs from + work_dir: "/work/lbogner/web" + + # Web hosting directory where gallery files are served (auto-served by KIT) + web_folder: "/web/lbogner/public_html/" + +gallery: + plot_root: "gallery" + png_dpi: 400 + backup_folder: "" + +ui: + max_recent_plots: 20 + search_debounce_ms: 300 + +metadata: + cache_enabled: true + inherit_from_parent: true + +# Data Sources +# TODO: fill in your actual plot-producing repo output directories under +# /work/lbogner/... and/or plot data under /ceph/... (adjust names/paths to match +# where each cloned repo's plotstyle scripts actually write their PDFs). +sources: + - name: "ttbar_analysis" + path: "/work/lbogner/PLACEHOLDER/ttbar_analysis/plots" + - name: "needle_benchmarks" + path: "/work/lbogner/PLACEHOLDER/needle/benchmarks/plots" + - name: "aido_convergence_study" + path: "/work/lbogner/PLACEHOLDER/aido/results_convergence/plots" diff --git a/deploy/systemd/README.md b/deploy/systemd/README.md new file mode 100644 index 0000000..048ec1c --- /dev/null +++ b/deploy/systemd/README.md @@ -0,0 +1,62 @@ +# Deploying on a KIT HPC login node (no Docker daemon) + +This path is for machines like the ETP login nodes: `/ceph`, `/work`, `/web` are +mounted directly, but there's no Docker daemon available, and your `public_html` +is already served by KIT's own web infrastructure — so no web server needs to +run here at all, only the generator on a schedule. + +## 1. Install into a venv + +```bash +python3 -m venv ~/.venvs/gallery +~/.venvs/gallery/bin/pip install -e /work/lbogner/ETPlot # path to your clone +``` + +PyMuPDF (a base dependency) handles PDF→PNG conversion; ImageMagick is not required. + +## 2. Point a config at your paths + +Copy/edit `config.lbogner.yaml` from the repo root (already has `web_folder: +/web/lbogner/public_html/` and placeholder `sources:` — fill in the real +plot-output directories under `/work/lbogner/...` or `/ceph/...`). Put it +wherever you like, e.g. `~/gallery/config.lbogner.yaml`, and adjust the path in +`gallery-generate.service` if you move it. + +## 3. Install the systemd --user timer + +```bash +mkdir -p ~/.config/systemd/user +cp deploy/systemd/gallery-generate.{service,timer} ~/.config/systemd/user/ +systemctl --user daemon-reload +systemctl --user enable --now gallery-generate.timer +``` + +Check status/logs: + +```bash +systemctl --user list-timers gallery-generate.timer +journalctl --user -u gallery-generate.service -f +``` + +By default a user's systemd instance (and its timers) stops when you log out. +Enable lingering so it keeps running: + +```bash +loginctl enable-linger $USER +``` + +If lingering isn't permitted on your login node, or `systemctl --user` isn't +usable there at all, fall back to a crontab entry instead: + +``` +*/5 * * * * ~/.venvs/gallery/bin/gallery --config ~/gallery/config.lbogner.yaml generate >> ~/gallery-generate.log 2>&1 +``` + +## Why not the Docker Compose stack? + +`docker-compose.yml` at the repo root (generator + nginx) is for a scenario +where you control a dedicated VM/server and need to serve the output yourself. +On the ETP login nodes there's no Docker daemon (Apptainer/Singularity only), +and `public_html` is already auto-served — running your own nginx there would +be redundant. Docker is still used for CI (`.gitlab-ci.yml`) and remains a fine +option for anyone deploying this on their own server. diff --git a/deploy/systemd/gallery-generate.service b/deploy/systemd/gallery-generate.service new file mode 100644 index 0000000..c0cf6df --- /dev/null +++ b/deploy/systemd/gallery-generate.service @@ -0,0 +1,7 @@ +[Unit] +Description=Generate ETPlot gallery +Wants=network-online.target + +[Service] +Type=oneshot +ExecStart=%h/.venvs/gallery/bin/gallery --config %h/gallery/config.lbogner.yaml generate --verbose diff --git a/deploy/systemd/gallery-generate.timer b/deploy/systemd/gallery-generate.timer new file mode 100644 index 0000000..3165e4a --- /dev/null +++ b/deploy/systemd/gallery-generate.timer @@ -0,0 +1,10 @@ +[Unit] +Description=Periodically regenerate the ETPlot gallery + +[Timer] +OnBootSec=2min +OnUnitActiveSec=5min +Persistent=true + +[Install] +WantedBy=timers.target diff --git a/gallery/__init__.py b/gallery/__init__.py index 3db6a1e..16d45ad 100644 --- a/gallery/__init__.py +++ b/gallery/__init__.py @@ -21,17 +21,23 @@ Example usage: __version__ = "0.1.0" __author__ = "K. Schmidt" +from gallery.api import generate +from gallery.builder import build_gallery, get_template from gallery.config import ( GalleryConfig, - GallerySource, GalleryDefaults, + GallerySource, ) -from gallery.api import generate - -# Export utility functions for testing and advanced usage -from gallery.utils.stats import ( - calculate_directory_stats, - format_file_size, +from gallery.utils.datetime_utils import ( + datetime_from_timestamp, + strftime_filter, +) +from gallery.utils.metadata import ( + load_folder_metadata, + load_metadata_file, + merge_metadata, + resolve_metadata_for_plot, + save_metadata_cache, ) from gallery.utils.processing import ( convert_pdf_to_png, @@ -39,18 +45,12 @@ from gallery.utils.processing import ( process_plot_files, render_gallery_page, ) -from gallery.utils.metadata import ( - load_folder_metadata, - merge_metadata, - save_metadata_cache, - load_metadata_file, - resolve_metadata_for_plot, + +# Export utility functions for testing and advanced usage +from gallery.utils.stats import ( + calculate_directory_stats, + format_file_size, ) -from gallery.utils.datetime_utils import ( - datetime_from_timestamp, - strftime_filter, -) -from gallery.builder import build_gallery, get_template __all__ = [ "generate", @@ -74,4 +74,3 @@ __all__ = [ "datetime_from_timestamp", "strftime_filter", ] - diff --git a/gallery/api.py b/gallery/api.py index 33e9b9e..7d3199a 100644 --- a/gallery/api.py +++ b/gallery/api.py @@ -6,19 +6,19 @@ Provides the primary entry point for programmatic gallery generation. import shutil from pathlib import Path -from typing import Union, List, Dict, Any +from typing import Any, Dict, List, Optional, Union, cast +from gallery.builder import build_gallery, copy_assets, get_template from gallery.config import GalleryConfig, GallerySource -from gallery.builder import get_template, build_gallery, copy_assets def generate( - config: Union[GalleryConfig, str, Path] = None, - web_folder: Union[str, Path] = None, - sources: List[Union[GallerySource, Dict[str, Any]]] = None, + config: Optional[Union[GalleryConfig, str, Path]] = None, + web_folder: Optional[Union[str, Path]] = None, + sources: Optional[List[Union[GallerySource, Dict[str, Any]]]] = None, clean_first: bool = False, verbose: bool = False, - source_to_update: GallerySource = None, + source_to_update: Optional[GallerySource] = None, ) -> bool: """ Generate a scientific gallery from plot sources. @@ -79,20 +79,11 @@ def generate( if isinstance(config, (str, Path)): config = GalleryConfig.from_yaml(config) elif not isinstance(config, GalleryConfig): - raise TypeError( - f"config must be GalleryConfig, str, or Path, " - f"got {type(config)}" - ) + raise TypeError(f"config must be GalleryConfig, str, or Path, got {type(config)}") else: if web_folder is None or sources is None: - raise ValueError( - "Either config or both web_folder and sources " - "must be provided" - ) - config = GalleryConfig( - web_folder=web_folder, - sources=sources or [] - ) + raise ValueError("Either config or both web_folder and sources must be provided") + config = GalleryConfig(web_folder=web_folder, sources=sources or []) # Validate configuration if not config.sources: @@ -104,10 +95,7 @@ def generate( web_folder_path = Path(config.web_folder) if not _is_writable(web_folder_path): if verbose: - print( - f"Error: Cannot write to web_folder: " - f"{config.web_folder}" - ) + print(f"Error: Cannot write to web_folder: {config.web_folder}") return False # Create gallery root directory @@ -127,17 +115,12 @@ def generate( source_subdir = gallery_root / source_to_update.name if source_subdir.exists(): if verbose: - print( - f"Updating source directory {source_to_update.name}..." - ) + print(f"Updating source directory {source_to_update.name}...") try: shutil.rmtree(source_subdir) except Exception as e: if verbose: - print( - f"Warning: Could not clean source subdirectory " - f"{source_subdir}: {e}" - ) + print(f"Warning: Could not clean source subdirectory {source_subdir}: {e}") return False try: @@ -162,8 +145,10 @@ def generate( return False # Process sources + # config.sources is always List[GallerySource] after GalleryConfig.__post_init__ normalizes it. source_subdirs = [] for source in config.sources: + source = cast(GallerySource, source) # Skip sources not matching the update target (if specified) if source_to_update and source.name != source_to_update.name: # Still include them in the index if they exist @@ -178,10 +163,7 @@ def generate( # Validate source exists if not source_path.exists(): if verbose: - print( - f"Warning: Source {source.path} does not exist. " - f"Skipping." - ) + print(f"Warning: Source {source.path} does not exist. Skipping.") continue source_web_dir = gallery_root / source.name @@ -189,47 +171,37 @@ def generate( source_web_dir.mkdir(parents=True, exist_ok=True) except Exception as e: if verbose: - print( - f"Warning: Could not create directory " - f"{source_web_dir}: {e}" - ) + print(f"Warning: Could not create directory {source_web_dir}: {e}") continue source_subdirs.append(source.name) # Process source - if source_path.is_file() and source_path.suffix == '.pdf': + if source_path.is_file() and source_path.suffix == ".pdf": # Single PDF file from gallery.utils.processing import process_plot_files + item = process_plot_files( config=config, plot_file=source_path, web_dir=source_web_dir, ) from gallery.utils.processing import render_gallery_page + render_gallery_page( config=config, template=template, web_dir=source_web_dir, items=[item], subdirs=[], - relative_path=Path(source.name) + relative_path=Path(source.name), ) elif source_path.is_dir(): # Directory of plots - build_gallery( - config, - source_path, - source_web_dir, - template, - Path(source.name) - ) + build_gallery(config, source_path, source_web_dir, template, Path(source.name)) else: if verbose: - print( - f"Warning: Source {source.path} is neither a " - f"directory nor a PDF file. Skipping." - ) + print(f"Warning: Source {source.path} is neither a directory nor a PDF file. Skipping.") continue if verbose: @@ -237,15 +209,13 @@ def generate( except Exception as e: if verbose: - print( - f"Warning: Error processing source " - f"{source.name}: {e}" - ) + print(f"Warning: Error processing source {source.name}: {e}") continue # Render gallery root index try: from gallery.utils.processing import render_gallery_page + render_gallery_page( config=config, template=template, @@ -253,7 +223,7 @@ def generate( items=[], subdirs=source_subdirs, relative_path=Path("."), - title="Gallery Root" + title="Gallery Root", ) except Exception as e: if verbose: @@ -277,8 +247,8 @@ def generate( def _count_gallery_plots(gallery_root: Path) -> int: """Recursively count plot files (PDFs and HTMLs, excluding index.html) in the gallery output.""" count = 0 - for f in gallery_root.rglob('*'): - if f.is_file() and f.suffix.lower() in ('.pdf', '.html') and f.name != 'index.html': + for f in gallery_root.rglob("*"): + if f.is_file() and f.suffix.lower() in (".pdf", ".html") and f.name != "index.html": count += 1 return count diff --git a/gallery/builder.py b/gallery/builder.py index 73d2cfb..ec39d1b 100644 --- a/gallery/builder.py +++ b/gallery/builder.py @@ -2,7 +2,8 @@ import shutil from pathlib import Path -from typing import Dict, Any, Optional, Union +from typing import Any, Dict, Optional, Union + from jinja2 import Environment, FileSystemLoader, Template from gallery.config import GalleryConfig @@ -17,7 +18,6 @@ from gallery.utils.metadata import ( ) from gallery.utils.processing import ( process_plot_files, - needs_update, render_gallery_page, ) @@ -39,13 +39,14 @@ def get_template(template_dir: Optional[Union[Path, str]] = None): if template_dir is None: # Use package-included template import gallery + gallery_module_path = Path(gallery.__file__).parent template_dir = gallery_module_path / "templates" env = Environment(loader=FileSystemLoader(str(template_dir))) - env.filters['datetime_from_timestamp'] = datetime_from_timestamp - env.filters['strftime'] = strftime_filter + env.filters["datetime_from_timestamp"] = datetime_from_timestamp + env.filters["strftime"] = strftime_filter return env.get_template("gallery.html") @@ -54,8 +55,8 @@ def build_gallery( config: GalleryConfig, source_dir: Path, web_dir: Path, - template: Template = None, - relative_path: Path = None, + template: Optional[Template] = None, + relative_path: Optional[Path] = None, inherited_metadata: Optional[Dict[str, Any]] = None, ) -> None: """ @@ -119,7 +120,7 @@ def build_gallery( subdir_web, template, subdir_relative, - current_metadata if config.inherit_from_parent else {} + current_metadata if config.inherit_from_parent else {}, ) subdir_names.append(subdir.name) @@ -130,15 +131,11 @@ def build_gallery( items=items, subdirs=subdir_names, relative_path=relative_path, - metadata=current_metadata + metadata=current_metadata, ) -def copy_assets( - config: GalleryConfig, - assets_src: Optional[Path] = None, - verbose: bool = False -) -> bool: +def copy_assets(config: GalleryConfig, assets_src: Optional[Path] = None, verbose: bool = False) -> bool: """ Copy assets to the web directory. @@ -154,14 +151,13 @@ def copy_assets( if assets_src is None: # Use package-included assets import gallery + gallery_module_path = Path(gallery.__file__).parent assets_src = gallery_module_path / "assets" if not assets_src.exists(): if verbose: - print( - f"Warning: Assets directory {assets_src} not found" - ) + print(f"Warning: Assets directory {assets_src} not found") return False gallery_root = Path(config.web_folder) / config.plot_root @@ -171,7 +167,7 @@ def copy_assets( # so any change to any JS/CSS file triggers a redeploy. sentinel_dst = assets_dst / "css" / "main.css" newest_src_mtime = max( - (f.stat().st_mtime for f in assets_src.rglob('*') if f.is_file()), + (f.stat().st_mtime for f in assets_src.rglob("*") if f.is_file()), default=0, ) dst_mtime = sentinel_dst.stat().st_mtime if sentinel_dst.exists() else 0 diff --git a/gallery/cli.py b/gallery/cli.py index 6aa747d..2b89ed1 100644 --- a/gallery/cli.py +++ b/gallery/cli.py @@ -10,13 +10,17 @@ import argparse import os import sys from pathlib import Path +from typing import List, Optional, cast import argcomplete from gallery import generate from gallery.config import ( - ConfigManager, GalleryConfig, GallerySource, - default_config_path, get_active_config_path, ensure_user_config, + ConfigManager, + GalleryConfig, + GallerySource, + ensure_user_config, + get_active_config_path, ) _WELCOME = """\ @@ -58,6 +62,11 @@ Config file: {config_path} """ +def _set_completer(action: argparse.Action, completer) -> None: + """argcomplete reads `.completer` dynamically; argparse.Action has no such attribute.""" + setattr(action, "completer", completer) + + def _is_configured(config_path: Path) -> bool: """Return True if web_folder is set to a non-empty value.""" try: @@ -104,13 +113,16 @@ def build_parser() -> argparse.ArgumentParser: description="Scientific Plot Gallery Generator", formatter_class=argparse.RawDescriptionHelpFormatter, ) - parser.add_argument( - "--config", - type=str, - default=None, - metavar="FILE", - help="Path to config file (default: package bundled config)", - ).completer = argcomplete.completers.FilesCompleter(["yaml", "yml"]) + _set_completer( + parser.add_argument( + "--config", + type=str, + default=None, + metavar="FILE", + help="Path to config file (default: package bundled config)", + ), + argcomplete.completers.FilesCompleter(["yaml", "yml"]), + ) sub = parser.add_subparsers(dest="command", metavar="COMMAND") @@ -121,13 +133,16 @@ def build_parser() -> argparse.ArgumentParser: description="Generate scientific gallery from plot collections", ) gen.add_argument("--clean", action="store_true", help="Clean gallery directory before generation") - gen.add_argument( - "--source", - type=str, - default=None, - metavar="DIR", - help="Only recompute a specific source directory (name defaults to dir name)", - ).completer = argcomplete.completers.DirectoriesCompleter() + _set_completer( + gen.add_argument( + "--source", + type=str, + default=None, + metavar="DIR", + help="Only recompute a specific source directory (name defaults to dir name)", + ), + argcomplete.completers.DirectoriesCompleter(), + ) gen.add_argument("-v", "--verbose", action="store_true", help="Print verbose output") # --- config ------------------------------------------------------------- @@ -146,20 +161,21 @@ def build_parser() -> argparse.ArgumentParser: cfg_sub.add_parser("sources", help="List all configured sources") p_get = cfg_sub.add_parser("get", help="Get a config value") - p_get.add_argument("key", help="Dot-separated key, e.g. gallery.png_dpi").completer = _config_keys + _set_completer(p_get.add_argument("key", help="Dot-separated key, e.g. gallery.png_dpi"), _config_keys) p_set = cfg_sub.add_parser("set", help="Set a config value") - p_set.add_argument("key", help="Dot-separated key, e.g. gallery.png_dpi").completer = _config_keys + _set_completer(p_set.add_argument("key", help="Dot-separated key, e.g. gallery.png_dpi"), _config_keys) p_set.add_argument("value", help="New value (YAML-parsed: use true/false for bools)") p_add = cfg_sub.add_parser("add-source", help="Add a plot source") p_add.add_argument("--name", default=None, help="Source name (default: bottom-level directory name)") - p_add.add_argument("--path", required=True, metavar="DIR", help="Path to source directory").completer = ( - argcomplete.completers.DirectoriesCompleter() + _set_completer( + p_add.add_argument("--path", required=True, metavar="DIR", help="Path to source directory"), + argcomplete.completers.DirectoriesCompleter(), ) p_rm = cfg_sub.add_parser("remove-source", help="Remove a plot source by name") - p_rm.add_argument("name", help="Source name to remove").completer = _source_names + _set_completer(p_rm.add_argument("name", help="Source name to remove"), _source_names) return parser @@ -174,10 +190,12 @@ def _run_generate(args: argparse.Namespace) -> int: try: config = GalleryConfig.from_yaml(config_path) - source_to_update = None + source_to_update: Optional[GallerySource] = None if args.source: source_path = Path(args.source).resolve() - matching = next((s for s in config.sources if Path(s.path).resolve() == source_path), None) + # config.sources is always List[GallerySource] after GalleryConfig.__post_init__ normalizes it. + typed_sources = cast(List[GallerySource], config.sources) + matching = next((s for s in typed_sources if Path(s.path).resolve() == source_path), None) if matching is None: source_to_update = GallerySource(name=source_path.name, path=source_path) config.sources.append(source_to_update) @@ -312,6 +330,7 @@ def main(): sys.exit(_run_install_completion()) elif args.command == "tui": from gallery.tui import GalleryTUI + config_path = Path(args.config) if args.config else None GalleryTUI(config_path=config_path).run() sys.exit(0) diff --git a/gallery/config/__init__.py b/gallery/config/__init__.py index 03f3a64..44f5426 100644 --- a/gallery/config/__init__.py +++ b/gallery/config/__init__.py @@ -8,7 +8,8 @@ defaults for gallery generation settings. import shutil from dataclasses import dataclass, field from pathlib import Path -from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List, Optional, Union, cast + import yaml from platformdirs import user_config_dir @@ -31,8 +32,8 @@ def user_config_path() -> Path: def get_active_config_path() -> Path: """Return the config path to use, with this precedence: - 1. User config (~/.config/gallery/config.yaml) — if it exists - 2. Package-bundled template — fallback + 1. User config (~/.config/gallery/config.yaml) — if it exists + 2. Package-bundled template — fallback """ ucp = user_config_path() return ucp if ucp.exists() else default_config_path() @@ -133,6 +134,7 @@ class ConfigManager: @dataclass class GalleryDefaults: """Default values for gallery generation.""" + png_dpi: int = 400 plot_root: str = "gallery" cache_enabled: bool = True @@ -142,6 +144,7 @@ class GalleryDefaults: @dataclass class GallerySource: """Represents a single data source for the gallery.""" + name: str path: Union[str, Path] @@ -157,6 +160,7 @@ class GalleryConfig: Can be created programmatically or loaded from YAML. """ + web_folder: Union[str, Path] sources: List[Union[GallerySource, Dict[str, Any]]] = field(default_factory=list) png_dpi: int = GalleryDefaults.png_dpi @@ -169,10 +173,11 @@ class GalleryConfig: if isinstance(self.web_folder, str): self.web_folder = Path(self.web_folder) - normalized_sources = [] + normalized_sources: List[Union[GallerySource, Dict[str, Any]]] = [] for source in self.sources: if isinstance(source, dict): - source = GallerySource(**source) + source_dict = cast(Dict[str, Any], source) + source = GallerySource(name=source_dict["name"], path=source_dict["path"]) elif not isinstance(source, GallerySource): raise TypeError(f"Source must be dict or GallerySource, got {type(source)}") normalized_sources.append(source) @@ -197,15 +202,19 @@ class GalleryConfig: gallery_cfg = data.get("gallery", {}) sources_data = data.get("sources", []) - sources = [{"name": s["name"], "path": s["path"]} for s in sources_data] + sources: List[Union[GallerySource, Dict[str, Any]]] = [ + {"name": s["name"], "path": s["path"]} for s in sources_data + ] + + metadata_cfg = data.get("metadata", {}) return cls( web_folder=web_folder, sources=sources, png_dpi=gallery_cfg.get("png_dpi", GalleryDefaults.png_dpi), plot_root=gallery_cfg.get("plot_root", GalleryDefaults.plot_root), - cache_enabled=data.get("metadata", {}).get("cache_enabled", GalleryDefaults.cache_enabled), - inherit_from_parent=data.get("metadata", {}).get("inherit_from_parent", GalleryDefaults.inherit_from_parent), + cache_enabled=metadata_cfg.get("cache_enabled", GalleryDefaults.cache_enabled), + inherit_from_parent=metadata_cfg.get("inherit_from_parent", GalleryDefaults.inherit_from_parent), backup_folder=gallery_cfg.get("backup_folder", ""), ) @@ -231,7 +240,8 @@ class GalleryConfig: "inherit_from_parent": self.inherit_from_parent, "supported_formats": [".yaml", ".yml", ".json"], }, - "sources": [{"name": s.name, "path": str(s.path)} for s in self.sources], + # self.sources is always List[GallerySource] after __post_init__ normalizes it. + "sources": [{"name": s.name, "path": str(s.path)} for s in cast(List[GallerySource], self.sources)], } with open(yaml_path, "w") as f: diff --git a/gallery/tui.py b/gallery/tui.py index 2d98555..d236611 100644 --- a/gallery/tui.py +++ b/gallery/tui.py @@ -27,6 +27,7 @@ from textual.app import App, ComposeResult from textual.binding import Binding from textual.containers import Horizontal, ScrollableContainer, Vertical from textual.reactive import reactive +from textual.widget import Widget from textual.widgets import Button, Collapsible, Footer, Header, Input, Label, RichLog, Static from gallery.config import ConfigManager, ensure_user_config, get_active_config_path @@ -37,12 +38,12 @@ from gallery.config import ConfigManager, ensure_user_config, get_active_config_ # widget_id is used as the HTML-style id (#web-folder) in CSS selectors # --------------------------------------------------------------------------- CONFIG_FIELDS = [ - ("web-folder", "paths.web_folder", True), - ("plot-root", "gallery.plot_root", False), - ("png-dpi", "gallery.png_dpi", False), - ("backup-folder", "gallery.backup_folder", False), - ("cache-enabled", "metadata.cache_enabled", False), - ("inherit-meta", "metadata.inherit_from_parent",False), + ("web-folder", "paths.web_folder", True), + ("plot-root", "gallery.plot_root", False), + ("png-dpi", "gallery.png_dpi", False), + ("backup-folder", "gallery.backup_folder", False), + ("cache-enabled", "metadata.cache_enabled", False), + ("inherit-meta", "metadata.inherit_from_parent", False), ] REQUIRED_IDS = {fid for fid, _, req in CONFIG_FIELDS if req} @@ -269,9 +270,9 @@ class GalleryTUI(App): # Bottom bar — always visible outside the scroll area with Horizontal(id="footer-bar"): yield Static("", id="dirty-indicator") - yield Button("Save Config", id="save-btn", variant="success") - yield Button("Generate", id="generate-btn", variant="primary") - yield Button("Quit", id="quit-btn", variant="error") + yield Button("Save Config", id="save-btn", variant="success") + yield Button("Generate", id="generate-btn", variant="primary") + yield Button("Quit", id="quit-btn", variant="error") yield Footer() @@ -280,9 +281,7 @@ class GalleryTUI(App): # ------------------------------------------------------------------ def on_mount(self) -> None: self._load_config_into_fields() - self.query_one("#config-path-label", Static).update( - f"Config: {self.config_path}" - ) + self.query_one("#config-path-label", Static).update(f"Config: {self.config_path}") def _make_source_row(self, name: str = "", path: str = "") -> Horizontal: """Return a single editable source row widget.""" @@ -368,7 +367,7 @@ class GalleryTUI(App): self._save_config() @on(Button.Pressed, "#quit-btn") - def action_quit(self) -> None: + async def action_quit(self) -> None: self.exit() @on(Button.Pressed, "#save-btn") @@ -421,7 +420,9 @@ class GalleryTUI(App): @on(Button.Pressed, ".source-remove-btn") def _remove_source_row(self, event: Button.Pressed) -> None: """Remove the row whose − button was pressed.""" - event.button.parent.remove() + parent = event.button.parent + assert isinstance(parent, Widget), "source-remove button must be mounted inside a source row widget" + parent.remove() self.dirty = True # -- Generate -------------------------------------------------------- @@ -461,8 +462,7 @@ class GalleryTUI(App): status.label = label # --config is a top-level flag (before the subcommand) in the CLI parser - cmd = [sys.executable, "-m", "gallery.cli", - "--config", str(self.config_path), "generate", "--verbose"] + cmd = [sys.executable, "-m", "gallery.cli", "--config", str(self.config_path), "generate", "--verbose"] try: proc = subprocess.Popen( cmd, @@ -470,6 +470,7 @@ class GalleryTUI(App): stderr=subprocess.STDOUT, text=True, ) + assert proc.stdout is not None, "Popen was called with stdout=PIPE" for line in proc.stdout: line = line.rstrip() if line: diff --git a/gallery/utils/backup.py b/gallery/utils/backup.py index 6396f1d..14ab225 100644 --- a/gallery/utils/backup.py +++ b/gallery/utils/backup.py @@ -1,14 +1,11 @@ """Backup utilities for gallery.""" -import zipfile import datetime +import zipfile from pathlib import Path -def create_backup( - web_folder: Path, - backup_folder: Path -) -> bool: +def create_backup(web_folder: Path, backup_folder: Path) -> bool: """ Create a backup of the web folder. diff --git a/gallery/utils/metadata.py b/gallery/utils/metadata.py index 964a38b..f42bc6a 100644 --- a/gallery/utils/metadata.py +++ b/gallery/utils/metadata.py @@ -13,9 +13,10 @@ Features: """ import json -import yaml from pathlib import Path -from typing import Dict, Any +from typing import Any, Dict + +import yaml def load_metadata_file(metadata_path: Path) -> Dict[str, Any]: @@ -33,15 +34,14 @@ def load_metadata_file(metadata_path: Path) -> Dict[str, Any]: return {} try: - with metadata_path.open('r', encoding='utf-8') as f: + with metadata_path.open("r", encoding="utf-8") as f: suffix_lower = metadata_path.suffix.lower() - if suffix_lower == '.yaml' or suffix_lower == '.yml': + if suffix_lower == ".yaml" or suffix_lower == ".yml": return yaml.safe_load(f) or {} - elif metadata_path.suffix.lower() == '.json': + elif metadata_path.suffix.lower() == ".json": return json.load(f) or {} else: - print(f"Warning: Unknown metadata file format: " - f"{metadata_path}") + print(f"Warning: Unknown metadata file format: {metadata_path}") return {} except (yaml.YAMLError, json.JSONDecodeError, IOError) as e: print(f"Warning: Could not parse metadata file {metadata_path}: {e}") @@ -59,7 +59,7 @@ def load_folder_metadata(folder_path: Path) -> Dict[str, Any]: Dictionary containing the folder metadata """ # Try YAML first, then JSON for backwards compatibility - for filename in ['metadata.yaml', 'metadata.yml', 'metadata.json']: + for filename in ["metadata.yaml", "metadata.yml", "metadata.json"]: metadata_path = folder_path / filename if metadata_path.exists(): @@ -81,7 +81,7 @@ def get_metadata_file_path(folder_path: Path) -> str: String path to the metadata file (existing or suggested) """ # Preferred order: YAML first, then JSON - preferred_files = ['metadata.yaml', 'metadata.yml', 'metadata.json'] + preferred_files = ["metadata.yaml", "metadata.yml", "metadata.json"] for filename in preferred_files: metadata_path = folder_path / filename @@ -89,13 +89,10 @@ def get_metadata_file_path(folder_path: Path) -> str: return str(metadata_path) # If no file exists, suggest metadata.yaml (preferred format) - return str(folder_path / 'metadata.yaml') + return str(folder_path / "metadata.yaml") -def merge_metadata( - parent_metadata: Dict[str, Any], - child_metadata: Dict[str, Any] -) -> Dict[str, Any]: +def merge_metadata(parent_metadata: Dict[str, Any], child_metadata: Dict[str, Any]) -> Dict[str, Any]: """ Merge parent and child metadata, with child values overriding parent. @@ -111,10 +108,7 @@ def merge_metadata( return merged -def resolve_metadata_for_plot( - plot_path: Path, - inherited_metadata: Dict[str, Any] -) -> Dict[str, Any]: +def resolve_metadata_for_plot(plot_path: Path, inherited_metadata: Dict[str, Any]) -> Dict[str, Any]: """ Resolve metadata for a specific plot. @@ -131,7 +125,7 @@ def resolve_metadata_for_plot( plot_dir = plot_path.parent # Check for plot-specific metadata files - for suffix in ['.yaml', '.yml', '.json']: + for suffix in [".yaml", ".yml", ".json"]: plot_metadata_path = plot_dir / f"{plot_stem}{suffix}" if plot_metadata_path.exists(): plot_metadata = load_metadata_file(plot_metadata_path) @@ -141,10 +135,7 @@ def resolve_metadata_for_plot( return inherited_metadata.copy() -def save_metadata_cache( - web_dir: Path, - plot_metadata_cache: Dict[str, Dict[str, Any]] -) -> None: +def save_metadata_cache(web_dir: Path, plot_metadata_cache: Dict[str, Dict[str, Any]]) -> None: """ Save plot metadata cache to meta_cache.json in the web directory. @@ -154,7 +145,7 @@ def save_metadata_cache( """ cache_path = web_dir / "meta_cache.json" try: - with cache_path.open('w', encoding='utf-8') as f: + with cache_path.open("w", encoding="utf-8") as f: json.dump(plot_metadata_cache, f, indent=2, ensure_ascii=False) except IOError as e: print(f"Warning: Could not save metadata cache {cache_path}: {e}") diff --git a/gallery/utils/processing.py b/gallery/utils/processing.py index b1e548d..3768174 100644 --- a/gallery/utils/processing.py +++ b/gallery/utils/processing.py @@ -3,34 +3,31 @@ import shutil import subprocess from pathlib import Path -from typing import Any, Dict +from typing import Any, Dict, Optional + +from jinja2 import Template + +from gallery.config import GalleryConfig +from gallery.utils.metadata import ( + get_metadata_file_path, + resolve_metadata_for_plot, +) +from gallery.utils.stats import ( + calculate_directory_stats, + format_file_size, +) try: import fitz # PyMuPDF + _PYMUPDF_AVAILABLE = True except ImportError: _PYMUPDF_AVAILABLE = False _IMAGEMAGICK_AVAILABLE = shutil.which("convert") is not None -from jinja2 import Template -from gallery.utils.metadata import ( - resolve_metadata_for_plot, - get_metadata_file_path, -) -from gallery.utils.stats import ( - calculate_directory_stats, - format_file_size, -) -from gallery.config import GalleryConfig - - -def process_html_file( - html_file: Path, - web_dir: Path, - current_metadata: Dict[str, Any] = None -) -> dict: +def process_html_file(html_file: Path, web_dir: Path, current_metadata: Optional[Dict[str, Any]] = None) -> dict: """ Process HTML plot file, copying it to web directory. @@ -60,15 +57,15 @@ def process_html_file( "html_href": html_file.name, "is_html": True, "metadata": plot_metadata, - "creation_time": source_creation_time + "creation_time": source_creation_time, } def process_plot_files( - config: GalleryConfig, - plot_file: Path, - web_dir: Path, - current_metadata: Dict[str, Any] = None, + config: GalleryConfig, + plot_file: Path, + web_dir: Path, + current_metadata: Optional[Dict[str, Any]] = None, ) -> dict: """ Process plot files (PDF/PNG or HTML), handling conversion and copying. @@ -82,7 +79,7 @@ def process_plot_files( Returns: Dictionary containing plot information """ - if plot_file.suffix.lower() == '.html': + if plot_file.suffix.lower() == ".html": return process_html_file(plot_file, web_dir, current_metadata) # Handle PDF files @@ -111,7 +108,7 @@ def process_plot_files( "png_href": png_file.name, "is_html": False, "metadata": plot_metadata, - "creation_time": source_creation_time + "creation_time": source_creation_time, } @@ -122,8 +119,8 @@ def render_gallery_page( items: list, subdirs: list, relative_path: Path, - title: str = None, - metadata: dict = None + title: Optional[str] = None, + metadata: Optional[dict] = None, ) -> None: """ Unified template rendering for all gallery pages. @@ -139,8 +136,7 @@ def render_gallery_page( metadata: Metadata dictionary (optional) """ if title is None: - title = "Gallery" if relative_path == Path( - ".") else f"Gallery: {relative_path}" + title = "Gallery" if relative_path == Path(".") else f"Gallery: {relative_path}" if metadata is None: metadata = {} @@ -151,7 +147,7 @@ def render_gallery_page( "file_count": len(items), "folder_count": len(subdirs), "total_size": format_file_size(current_stats["total_size"]), - "total_size_bytes": current_stats["total_size"] + "total_size_bytes": current_stats["total_size"], } # Calculate relative path to assets @@ -185,7 +181,7 @@ def render_gallery_page( folder_metadata=metadata, assets_path=assets_path, source_dir=str(web_dir), - metadata_file_path=get_metadata_file_path(web_dir) + metadata_file_path=get_metadata_file_path(web_dir), ) f.write(rendered_html) @@ -232,13 +228,18 @@ def _convert_pdf_pymupdf(pdf_path: Path, png_path: Path, dpi: int) -> None: def _convert_pdf_imagemagick(pdf_path: Path, png_path: Path, dpi: int) -> None: - subprocess.run([ - "convert", - "-density", str(dpi), - str(pdf_path), - "-quality", "95", - str(png_path), - ], check=True) + subprocess.run( + [ + "convert", + "-density", + str(dpi), + str(pdf_path), + "-quality", + "95", + str(png_path), + ], + check=True, + ) def needs_update(source_file: Path, target_file: Path) -> bool: diff --git a/gallery/utils/stats.py b/gallery/utils/stats.py index 0af246d..6f79f06 100644 --- a/gallery/utils/stats.py +++ b/gallery/utils/stats.py @@ -30,9 +30,9 @@ def calculate_directory_stats(directory: Path) -> dict: size = item.stat().st_size stats["total_size"] += size - if item.suffix.lower() == '.pdf': + if item.suffix.lower() == ".pdf": stats["pdf_size"] += size - elif item.suffix.lower() == '.png': + elif item.suffix.lower() == ".png": stats["png_size"] += size elif item.is_dir(): stats["folder_count"] += 1 diff --git a/plotstyle/annotations.py b/plotstyle/annotations.py index 3671618..34fc31e 100644 --- a/plotstyle/annotations.py +++ b/plotstyle/annotations.py @@ -3,6 +3,7 @@ from __future__ import annotations import warnings +from typing import Optional from matplotlib.axes import Axes from matplotlib.colors import to_rgba @@ -21,7 +22,7 @@ def style_legend( ax: Axes, loc: str = "outside right upper", frameon: bool = False, - title: str = None, + title: Optional[str] = None, **kwargs, ): """Add a figure-level legend, placed outside the axes by default. @@ -43,12 +44,15 @@ def style_legend( ) fig = ax.get_figure() + assert fig is not None, "ax must be attached to a figure" handles = kwargs.pop("handles", None) labels = kwargs.pop("labels", None) if handles is None or labels is None: handles, labels = ax.get_legend_handles_labels() - legend = fig.legend(handles, labels, loc=loc, frameon=frameon, title=title, **kwargs) + # matplotlib-stubs' `loc` Literal doesn't include the "outside ..." compound + # locations matplotlib actually supports at runtime (e.g. "outside right upper"). + legend = fig.legend(handles, labels, loc=loc, frameon=frameon, title=title, **kwargs) # ty: ignore[invalid-argument-type] if legend.get_title() is not None: legend.get_title().set_fontweight("bold") return legend diff --git a/plotstyle/figures.py b/plotstyle/figures.py index 6f67932..894cd53 100644 --- a/plotstyle/figures.py +++ b/plotstyle/figures.py @@ -5,8 +5,8 @@ from __future__ import annotations from pathlib import Path from typing import Mapping, Sequence, Union -import numpy as np import matplotlib.pyplot as plt +import numpy as np from matplotlib.axes import Axes from matplotlib.colorbar import Colorbar from matplotlib.figure import Figure @@ -53,8 +53,8 @@ def _set_figure_title(fig: Figure, title: Union[str, None], params: Union[Mappin def new_figure( preset: Union[str, tuple] = "thesis-single", *, - title: str = None, - params: Mapping = None, + title: Union[str, None] = None, + params: Union[Mapping, None] = None, **subplots_kwargs, ): """Create a figure/axes pair sized for a named preset or an explicit (w, h) tuple. @@ -113,7 +113,9 @@ def colorbar(mappable, ax: Axes, size: str = "5%", pad: float = 0.05, **kwargs) """ divider = make_axes_locatable(ax) cax = divider.append_axes("right", size=size, pad=pad) - cb = ax.get_figure().colorbar(mappable, cax=cax, **kwargs) + fig = ax.get_figure() + assert fig is not None, "ax must be attached to a figure" + cb = fig.colorbar(mappable, cax=cax, **kwargs) # Colorbar draws its own border (a dedicated "outline" spine) that isn't # covered by axes.spines.{left,right,top} — without this it'd pick up # the bold black bottom-spine color/width from the main theme as a box diff --git a/plotstyle/style.py b/plotstyle/style.py index e73b57e..c990d89 100644 --- a/plotstyle/style.py +++ b/plotstyle/style.py @@ -20,10 +20,10 @@ exactly this reason. from __future__ import annotations -from cycler import cycler from importlib import resources import matplotlib.pyplot as plt +from cycler import cycler from .colors import CATEGORICAL @@ -45,7 +45,7 @@ def use(cycle_linestyles: bool = False) -> None: sequence, so series stay distinguishable even if color is lost (grayscale printing, projector glare, color-vision deficiency). """ - style_path = resources.files("plotstyle").joinpath("assets", "plotstyle.mplstyle") + style_path = resources.files("plotstyle").joinpath("assets").joinpath("plotstyle.mplstyle") plt.style.use(str(style_path)) if cycle_linestyles: diff --git a/pyproject.toml b/pyproject.toml index ccf629f..4b80c1d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -46,9 +46,9 @@ dependencies = [ [project.optional-dependencies] dev = [ "pytest>=7.0", - "black>=22.0", - "pylint>=2.0", - "mypy>=0.900", + "ruff>=0.6", + "ty>=0.0.1", + "pip-audit>=2.7", ] plotting = [ "matplotlib>=3.7", @@ -62,14 +62,9 @@ packages = ["gallery", "gallery.utils", "gallery.config", "plotstyle"] package-data = {gallery = ["templates/*", "assets/css/*", "assets/js/*", "config/*"], plotstyle = ["assets/*.mplstyle"]} include-package-data = true -[tool.black] +[tool.ruff] line-length = 120 -target-version = ['py38'] +target-version = "py38" -[tool.isort] -profile = "black" -line_length = 120 - -[tool.flake8] -max-line-length = 120 -extend-ignore = ["E203", "W503"] +[tool.ruff.lint] +select = ["E", "F", "I"] diff --git a/tests/__init__.py b/tests/__init__.py index 2889ef3..4646851 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,4 +1,3 @@ import sys - sys.path.append("..") diff --git a/tests/test_backup.py b/tests/test_backup.py index 635bed4..9890cce 100644 --- a/tests/test_backup.py +++ b/tests/test_backup.py @@ -1,214 +1,137 @@ -import zipfile import datetime +import zipfile from unittest.mock import patch -from utils import backup + +from gallery.utils.backup import create_backup -def test_backup_creates_zip(tmp_path, monkeypatch): - # Setup fake web folder - web_folder = tmp_path / 'plots' +def test_backup_creates_zip(tmp_path): + web_folder = tmp_path / "plots" web_folder.mkdir() - (web_folder / 'file1.txt').write_text('abc') - (web_folder / 'file2.txt').write_text('def') - backup_folder = tmp_path / 'backups' + (web_folder / "file1.txt").write_text("abc") + (web_folder / "file2.txt").write_text("def") + backup_folder = tmp_path / "backups" backup_folder.mkdir() - # Patch the module variables - monkeypatch.setattr(backup, 'WEB_FOLDER', web_folder) - monkeypatch.setattr(backup, 'BACKUP_FOLDER', backup_folder) + assert create_backup(web_folder, backup_folder) is True - # Call the backup function - backup.create_backup() - - # Check that backup was created - today = datetime.date.today().strftime('%Y%m%d') - backup_name = f'backup-{today}.zip' - backup_path = backup_folder / backup_name + today = datetime.date.today().strftime("%Y%m%d") + backup_path = backup_folder / f"backup-{today}.zip" assert backup_path.exists() - with zipfile.ZipFile(backup_path, 'r') as z: + with zipfile.ZipFile(backup_path, "r") as z: names = z.namelist() - assert any('file1.txt' in n for n in names) - assert any('file2.txt' in n for n in names) - - # Cleanup: remove the backup file after test - backup_path.unlink() + assert any("file1.txt" in n for n in names) + assert any("file2.txt" in n for n in names) -def test_backup_with_subdirectories(tmp_path, monkeypatch): - # Setup fake web folder with subdirectories - web_folder = tmp_path / 'plots' +def test_backup_with_subdirectories(tmp_path): + web_folder = tmp_path / "plots" web_folder.mkdir() - (web_folder / 'file1.txt').write_text('content1') - - subdir = web_folder / 'subdir' + (web_folder / "file1.txt").write_text("content1") + + subdir = web_folder / "subdir" subdir.mkdir() - (subdir / 'file2.txt').write_text('content2') - - nested_subdir = subdir / 'nested' + (subdir / "file2.txt").write_text("content2") + + nested_subdir = subdir / "nested" nested_subdir.mkdir() - (nested_subdir / 'file3.txt').write_text('content3') - - backup_folder = tmp_path / 'backups' + (nested_subdir / "file3.txt").write_text("content3") + + backup_folder = tmp_path / "backups" backup_folder.mkdir() - # Patch the module variables - monkeypatch.setattr(backup, 'WEB_FOLDER', web_folder) - monkeypatch.setattr(backup, 'BACKUP_FOLDER', backup_folder) + assert create_backup(web_folder, backup_folder) is True - # Call the backup function - backup.create_backup() - - # Check that backup was created with all files - today = datetime.date.today().strftime('%Y%m%d') - backup_name = f'backup-{today}.zip' - backup_path = backup_folder / backup_name + today = datetime.date.today().strftime("%Y%m%d") + backup_path = backup_folder / f"backup-{today}.zip" assert backup_path.exists() - with zipfile.ZipFile(backup_path, 'r') as z: + with zipfile.ZipFile(backup_path, "r") as z: names = z.namelist() - assert any('file1.txt' in n for n in names) - assert any('file2.txt' in n for n in names) - assert any('file3.txt' in n for n in names) - - # Cleanup - backup_path.unlink() + assert any("file1.txt" in n for n in names) + assert any("file2.txt" in n for n in names) + assert any("file3.txt" in n for n in names) -def test_backup_existing_file(tmp_path, monkeypatch, capsys): - # Setup fake web folder - web_folder = tmp_path / 'plots' +def test_backup_existing_file_is_not_overwritten(tmp_path): + web_folder = tmp_path / "plots" web_folder.mkdir() - (web_folder / 'file1.txt').write_text('abc') - - backup_folder = tmp_path / 'backups' + (web_folder / "file1.txt").write_text("abc") + + backup_folder = tmp_path / "backups" backup_folder.mkdir() - # Create existing backup file - today = datetime.date.today().strftime('%Y%m%d') - backup_name = f'backup-{today}.zip' - backup_path = backup_folder / backup_name - backup_path.write_text('existing backup') + today = datetime.date.today().strftime("%Y%m%d") + backup_path = backup_folder / f"backup-{today}.zip" + backup_path.write_text("existing backup") - # Patch the module variables - monkeypatch.setattr(backup, 'WEB_FOLDER', web_folder) - monkeypatch.setattr(backup, 'BACKUP_FOLDER', backup_folder) - - # Call the backup function - backup.create_backup() - - # Check that message about existing backup was printed - captured = capsys.readouterr() - assert f"Backup already exists: {backup_path}" in captured.out - - # Cleanup - backup_path.unlink() + assert create_backup(web_folder, backup_folder) is True + assert backup_path.read_text() == "existing backup" -def test_backup_empty_folder(tmp_path, monkeypatch): - # Setup empty web folder - web_folder = tmp_path / 'plots' +def test_backup_empty_folder(tmp_path): + web_folder = tmp_path / "plots" web_folder.mkdir() - - backup_folder = tmp_path / 'backups' + + backup_folder = tmp_path / "backups" backup_folder.mkdir() - # Patch the module variables - monkeypatch.setattr(backup, 'WEB_FOLDER', web_folder) - monkeypatch.setattr(backup, 'BACKUP_FOLDER', backup_folder) + assert create_backup(web_folder, backup_folder) is True - # Call the backup function - backup.create_backup() - - # Check that backup was created (empty) - today = datetime.date.today().strftime('%Y%m%d') - backup_name = f'backup-{today}.zip' - backup_path = backup_folder / backup_name + today = datetime.date.today().strftime("%Y%m%d") + backup_path = backup_folder / f"backup-{today}.zip" assert backup_path.exists() - with zipfile.ZipFile(backup_path, 'r') as z: + with zipfile.ZipFile(backup_path, "r") as z: assert len(z.namelist()) == 0 - - # Cleanup - backup_path.unlink() -def test_backup_nonexistent_web_folder(tmp_path, monkeypatch): - # Setup nonexistent web folder - web_folder = tmp_path / 'nonexistent_plots' - backup_folder = tmp_path / 'backups' +def test_backup_nonexistent_web_folder(tmp_path): + web_folder = tmp_path / "nonexistent_plots" + backup_folder = tmp_path / "backups" backup_folder.mkdir() - # Patch the module variables - monkeypatch.setattr(backup, 'WEB_FOLDER', web_folder) - monkeypatch.setattr(backup, 'BACKUP_FOLDER', backup_folder) + assert create_backup(web_folder, backup_folder) is True - # Call the backup function - backup.create_backup() - - # Check that backup was created (empty since source doesn't exist) - today = datetime.date.today().strftime('%Y%m%d') - backup_name = f'backup-{today}.zip' - backup_path = backup_folder / backup_name + today = datetime.date.today().strftime("%Y%m%d") + backup_path = backup_folder / f"backup-{today}.zip" assert backup_path.exists() - with zipfile.ZipFile(backup_path, 'r') as z: + with zipfile.ZipFile(backup_path, "r") as z: assert len(z.namelist()) == 0 - - # Cleanup - backup_path.unlink() -@patch('datetime.date') -def test_backup_with_custom_date(mock_date, tmp_path, monkeypatch): - # Mock date to return a specific date +@patch("datetime.date") +def test_backup_with_custom_date(mock_date, tmp_path): mock_date.today.return_value.strftime.return_value = "20230908" - - # Setup fake web folder - web_folder = tmp_path / 'plots' + + web_folder = tmp_path / "plots" web_folder.mkdir() - (web_folder / 'file1.txt').write_text('test') - - backup_folder = tmp_path / 'backups' + (web_folder / "file1.txt").write_text("test") + + backup_folder = tmp_path / "backups" backup_folder.mkdir() - # Patch the module variables - monkeypatch.setattr(backup, 'WEB_FOLDER', web_folder) - monkeypatch.setattr(backup, 'BACKUP_FOLDER', backup_folder) + assert create_backup(web_folder, backup_folder) is True - # Call the backup function - backup.create_backup() - - # Check that backup was created with custom date backup_path = backup_folder / "backup-20230908.zip" assert backup_path.exists() - - # Cleanup - backup_path.unlink() -def test_backup_folder_creation(tmp_path, monkeypatch): - # Setup fake web folder - web_folder = tmp_path / 'plots' +def test_backup_folder_creation(tmp_path): + web_folder = tmp_path / "plots" web_folder.mkdir() - (web_folder / 'file1.txt').write_text('test') - - # Don't create backup folder - let function create it - backup_folder = tmp_path / 'new_backups' + (web_folder / "file1.txt").write_text("test") - # Patch the module variables - monkeypatch.setattr(backup, 'WEB_FOLDER', web_folder) - monkeypatch.setattr(backup, 'BACKUP_FOLDER', backup_folder) + # Don't create backup folder - let create_backup() create it + backup_folder = tmp_path / "new_backups" + + assert create_backup(web_folder, backup_folder) is True - # Call the backup function - backup.create_backup() - - # Check that backup folder was created assert backup_folder.exists() assert backup_folder.is_dir() - - # Check that backup file was created - today = datetime.date.today().strftime('%Y%m%d') - backup_name = f'backup-{today}.zip' - backup_path = backup_folder / backup_name - assert backup_path.exists() \ No newline at end of file + + today = datetime.date.today().strftime("%Y%m%d") + backup_path = backup_folder / f"backup-{today}.zip" + assert backup_path.exists() diff --git a/tests/test_config.py b/tests/test_config.py index e8df5ea..c776aa0 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,195 +1,187 @@ from pathlib import Path -import tempfile + import pytest import yaml -from utils import config + +from gallery.config import ConfigManager, GalleryConfig, GalleryDefaults, GallerySource -def test_path_config(): - pc = config.PathConfig(work_dir='/tmp', web_folder='/web') - assert pc.work_dir == '/tmp' - assert pc.web_folder == '/web' +def test_gallery_source_path_conversion(): + source = GallerySource(name="test", path="/test/path") + assert source.name == "test" + assert source.path == Path("/test/path") -def test_gallery_config(): - gc = config.GalleryConfig( - plot_root='plots', png_dpi=150, backup_folder='backups') - assert gc.plot_root == 'plots' - assert gc.png_dpi == 150 - assert gc.backup_folder == 'backups' +def test_gallery_defaults(): + defaults = GalleryDefaults() + assert defaults.png_dpi == 400 + assert defaults.plot_root == "gallery" + assert defaults.cache_enabled is True + assert defaults.inherit_from_parent is True -def test_ui_config(): - ui = config.UIConfig(max_recent_plots=10, search_debounce_ms=200) - assert ui.max_recent_plots == 10 - assert ui.search_debounce_ms == 200 +def test_gallery_config_defaults(): + cfg = GalleryConfig(web_folder="/web") + assert cfg.web_folder == Path("/web") + assert cfg.sources == [] + assert cfg.png_dpi == GalleryDefaults.png_dpi + assert cfg.plot_root == GalleryDefaults.plot_root + assert cfg.cache_enabled == GalleryDefaults.cache_enabled + assert cfg.inherit_from_parent == GalleryDefaults.inherit_from_parent + assert cfg.backup_folder == "" -def test_metadata_config_defaults(): - mc = config.MetadataConfig() - assert mc.cache_enabled is True - assert mc.inherit_from_parent is True - assert mc.supported_formats == ['.yaml', '.yml', '.json'] +def test_gallery_config_sources_from_dicts(): + cfg = GalleryConfig(web_folder="/web", sources=[{"name": "s1", "path": "/p1"}]) + assert len(cfg.sources) == 1 + assert isinstance(cfg.sources[0], GallerySource) + assert cfg.sources[0].name == "s1" + assert cfg.sources[0].path == Path("/p1") -def test_metadata_config_custom(): - mc = config.MetadataConfig( - cache_enabled=False, - inherit_from_parent=False, - supported_formats=['.yaml'] - ) - assert mc.cache_enabled is False - assert mc.inherit_from_parent is False - assert mc.supported_formats == ['.yaml'] +def test_gallery_config_sources_invalid_type(): + with pytest.raises(TypeError): + GalleryConfig(web_folder="/web", sources=[123]) -def test_gallery_item(): - item = config.GalleryItem(name="test", path=Path("/test/path")) - assert item.name == "test" - assert item.path == Path("/test/path") +def test_gallery_config_from_yaml(tmp_path): + yaml_content = { + "paths": {"web_folder": "/test/web"}, + "gallery": {"plot_root": "test_plots", "png_dpi": 200, "backup_folder": "test_backups"}, + "metadata": {"cache_enabled": False, "inherit_from_parent": False}, + "sources": [ + {"name": "source1", "path": "/path1"}, + {"name": "source2", "path": "/path2"}, + ], + } + + yaml_file = tmp_path / "test_config.yaml" + with yaml_file.open("w") as f: + yaml.dump(yaml_content, f) + + cfg = GalleryConfig.from_yaml(yaml_file) + + assert cfg.web_folder == Path("/test/web") + assert cfg.plot_root == "test_plots" + assert cfg.png_dpi == 200 + assert cfg.backup_folder == "test_backups" + assert cfg.cache_enabled is False + assert cfg.inherit_from_parent is False + assert len(cfg.sources) == 2 + assert cfg.sources[0].name == "source1" + assert cfg.sources[0].path == Path("/path1") -def test_config_creation(): - paths = config.PathConfig(work_dir="/work", web_folder="/web") - gallery = config.GalleryConfig( - plot_root="plots", png_dpi=300, backup_folder="backups") - ui = config.UIConfig(max_recent_plots=5, search_debounce_ms=100) - metadata = config.MetadataConfig() +def test_gallery_config_from_yaml_missing_file(): + with pytest.raises(FileNotFoundError): + GalleryConfig.from_yaml("/nonexistent/file.yaml") - cfg = config.Config( - paths=paths, - gallery=gallery, - ui=ui, - metadata=metadata - ) - assert cfg.paths == paths - assert cfg.gallery == gallery - assert cfg.ui == ui - assert cfg.metadata == metadata +def test_gallery_config_from_yaml_missing_web_folder(tmp_path): + yaml_file = tmp_path / "no_web_folder.yaml" + yaml_file.write_text(yaml.dump({"gallery": {"plot_root": "plots"}})) + + with pytest.raises(ValueError): + GalleryConfig.from_yaml(yaml_file) + + +def test_gallery_config_from_yaml_partial_data(tmp_path): + yaml_content = {"paths": {"web_folder": "/min_web"}} + + yaml_file = tmp_path / "minimal_config.yaml" + yaml_file.write_text(yaml.dump(yaml_content)) + + cfg = GalleryConfig.from_yaml(yaml_file) + + assert cfg.web_folder == Path("/min_web") + assert cfg.png_dpi == GalleryDefaults.png_dpi + assert cfg.plot_root == GalleryDefaults.plot_root + assert cfg.cache_enabled is True assert cfg.sources == [] -def test_config_backward_compatibility_properties(): - paths = config.PathConfig(work_dir="/work", web_folder="/web") - gallery = config.GalleryConfig( - plot_root="plots", png_dpi=300, backup_folder="backups") - ui = config.UIConfig(max_recent_plots=5, search_debounce_ms=100) - metadata = config.MetadataConfig() - - cfg = config.Config( - paths=paths, - gallery=gallery, - ui=ui, - metadata=metadata +def test_gallery_config_to_yaml_round_trip(tmp_path): + cfg = GalleryConfig( + web_folder="/web", + sources=[{"name": "test", "path": "/test"}], + plot_root="plots", + png_dpi=300, + backup_folder="backups", ) - assert cfg.web_folder == "/web" - assert cfg.png_dpi == 300 - assert cfg.plot_root == "plots" - assert cfg.backup_folder == "backups" - - -def test_config_from_yaml(tmp_path): - yaml_content = { - 'paths': { - 'work_dir': '/test/work', - 'web_folder': '/test/web' - }, - 'gallery': { - 'plot_root': 'test_plots', - 'png_dpi': 200, - 'backup_folder': 'test_backups' - }, - 'ui': { - 'max_recent_plots': 15, - 'search_debounce_ms': 300 - }, - 'metadata': { - 'cache_enabled': False, - 'inherit_from_parent': False - }, - 'sources': [ - {'name': 'source1', 'path': '/path1'}, - {'name': 'source2', 'path': '/path2'} - ] - } - - yaml_file = tmp_path / 'test_config.yaml' - with yaml_file.open('w') as f: - yaml.dump(yaml_content, f) - - cfg = config.Config.from_yaml(str(yaml_file)) - - assert cfg.paths.work_dir == '/test/work' - assert cfg.paths.web_folder == '/test/web' - assert cfg.gallery.plot_root == 'test_plots' - assert cfg.gallery.png_dpi == 200 - assert cfg.ui.max_recent_plots == 15 - assert cfg.metadata.cache_enabled is False - assert len(cfg.sources) == 2 - assert cfg.sources[0].name == 'source1' - assert cfg.sources[0].path == Path('/path1') - - -def test_config_from_yaml_missing_file(): - with pytest.raises(FileNotFoundError): - config.Config.from_yaml('/nonexistent/file.yaml') - - -def test_config_from_yaml_malformed(): - with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml', - delete=False) as f: - f.write('invalid: yaml: content: [') - f.flush() - - with pytest.raises(yaml.YAMLError): - config.Config.from_yaml(f.name) - - -def test_config_to_yaml(tmp_path): - paths = config.PathConfig(work_dir="/work", web_folder="/web") - gallery = config.GalleryConfig( - plot_root="plots", png_dpi=300, backup_folder="backups") - ui = config.UIConfig(max_recent_plots=5, search_debounce_ms=100) - metadata = config.MetadataConfig() - sources = [config.GalleryItem(name="test", path=Path("/test"))] - - cfg = config.Config( - paths=paths, - gallery=gallery, - ui=ui, - metadata=metadata, - sources=sources - ) - - yaml_file = tmp_path / 'output_config.yaml' - cfg.to_yaml(str(yaml_file)) + yaml_file = tmp_path / "output_config.yaml" + cfg.to_yaml(yaml_file) assert yaml_file.exists() - # Just verify the file contains expected content (no Path parsing) - content = yaml_file.read_text() - assert 'work_dir: /work' in content - assert 'png_dpi: 300' in content - assert 'name: test' in content + reloaded = GalleryConfig.from_yaml(yaml_file) + assert reloaded.web_folder == cfg.web_folder + assert reloaded.plot_root == cfg.plot_root + assert reloaded.png_dpi == cfg.png_dpi + assert reloaded.backup_folder == cfg.backup_folder + assert reloaded.sources[0].name == "test" -def test_config_from_yaml_partial_data(tmp_path): - # Test with minimal YAML data - yaml_content = { - 'paths': {'work_dir': '/min', 'web_folder': '/min_web'}, - 'gallery': {'plot_root': 'min_plots', 'png_dpi': 100, - 'backup_folder': 'min_backup'}, - 'ui': {'max_recent_plots': 3, 'search_debounce_ms': 50} - } +# --------------------------------------------------------------------------- +# ConfigManager +# --------------------------------------------------------------------------- - yaml_file = tmp_path / 'minimal_config.yaml' - with yaml_file.open('w') as f: - yaml.dump(yaml_content, f) - cfg = config.Config.from_yaml(str(yaml_file)) +@pytest.fixture +def config_manager(tmp_path): + path = tmp_path / "config.yaml" + path.write_text( + yaml.dump( + { + "paths": {"web_folder": "/web"}, + "gallery": {"plot_root": "gallery", "png_dpi": 400}, + "sources": [{"name": "existing", "path": "/existing"}], + } + ) + ) + return ConfigManager(path) - # Should use defaults for metadata and empty sources - assert cfg.metadata.cache_enabled is True # default - assert cfg.sources == [] # default empty list + +def test_config_manager_get(config_manager): + assert config_manager.get("gallery.png_dpi") == 400 + + +def test_config_manager_get_missing_key(config_manager): + with pytest.raises(KeyError): + config_manager.get("gallery.nonexistent") + + +def test_config_manager_set(config_manager): + config_manager.set("gallery.png_dpi", "600") + assert config_manager.get("gallery.png_dpi") == 600 + + +def test_config_manager_list_all(config_manager): + data = config_manager.list_all() + assert data["paths"]["web_folder"] == "/web" + + +def test_config_manager_add_source(config_manager): + config_manager.add_source("new_source", "/new/path") + sources = config_manager.list_sources() + assert {"name": "new_source", "path": "/new/path"} in sources + + +def test_config_manager_add_source_duplicate(config_manager): + with pytest.raises(ValueError): + config_manager.add_source("existing", "/other/path") + + +def test_config_manager_remove_source(config_manager): + config_manager.remove_source("existing") + assert config_manager.list_sources() == [] + + +def test_config_manager_remove_source_not_found(config_manager): + with pytest.raises(KeyError): + config_manager.remove_source("nonexistent") + + +def test_config_manager_list_sources(config_manager): + sources = config_manager.list_sources() + assert sources == [{"name": "existing", "path": "/existing"}] diff --git a/tests/test_container.py b/tests/test_container.py deleted file mode 100644 index de0451d..0000000 --- a/tests/test_container.py +++ /dev/null @@ -1,138 +0,0 @@ -import pytest -import tempfile -import time -import os -from pathlib import Path -import subprocess -import sys - - -def test_python_version(): - """Test that Python 3.9+ is available.""" - version = sys.version_info - assert version.major >= 3 - assert version.minor >= 9 - - -def test_required_modules(): - """Test that required Python modules are installed.""" - try: - import jinja2 # noqa: F401 - import yaml # noqa: F401 - except ImportError as e: - pytest.fail(f"Required module not found: {e}") - - -def test_imagemagick_available(): - """Test that ImageMagick is installed and accessible.""" - try: - result = subprocess.run(['convert', '-version'], capture_output=True, text=True, timeout=10) - assert result.returncode == 0 - assert 'ImageMagick' in result.stdout - except (subprocess.TimeoutExpired, FileNotFoundError): - pytest.fail("ImageMagick not available or not working") - - -def test_format_file_size(): - # Add current directory to path instead of /src - sys.path.insert(0, '.') - from gallery import format_file_size - assert format_file_size(0) == "0 B" - assert format_file_size(1024) == "1.0 KB" - assert format_file_size(1048576) == "1.0 MB" - assert format_file_size(1073741824) == "1.0 GB" - - -def test_needs_update(): - sys.path.insert(0, '.') - from gallery import needs_update - with tempfile.TemporaryDirectory() as temp_dir: - temp_path = Path(temp_dir) - source = temp_path / "source.txt" - target = temp_path / "target.txt" - source.write_text("test") - assert needs_update(source, target) - target.write_text("test") - time.sleep(0.1) - os.utime(target, (time.time(), time.time())) - assert not needs_update(source, target) - - -def test_metadata_loading(): - sys.path.insert(0, '.') - from utils.metadata import load_metadata_file - with tempfile.TemporaryDirectory() as temp_dir: - temp_path = Path(temp_dir) - yaml_file = temp_path / "test.yaml" - yaml_content = "title: Test\nauthor: Container Test\n" - yaml_file.write_text(yaml_content) - metadata = load_metadata_file(yaml_file) - assert metadata['title'] == 'Test' - assert metadata['author'] == 'Container Test' - - -def test_metadata_inheritance(): - sys.path.insert(0, '.') - from utils.metadata import merge_metadata - parent = {'project': 'Test', 'version': '1.0'} - child = {'experiment': 'A', 'version': '1.1'} - merged = merge_metadata(parent, child) - assert merged['project'] == 'Test' - assert merged['experiment'] == 'A' - assert merged['version'] == '1.1' - - -def create_mock_pdf(path: Path): - path.write_text("%PDF-1.4\nMock PDF for testing") - - -def test_pdf_conversion(tmpdir): - sys.path.insert(0, '/src') - from gallery import convert_pdf_to_png, GalleryConfig - with tempfile.TemporaryDirectory() as temp_dir: - temp_path = Path(temp_dir) - pdf_path = temp_path / "test.pdf" - create_mock_pdf(pdf_path) - try: - convert_pdf_to_png(pdf_path, GalleryConfig(tmpdir)) - png_path = pdf_path.with_suffix('.png') - assert png_path.exists() - except subprocess.CalledProcessError: - pytest.skip("Mock PDF not processable by ImageMagick") - - -def create_test_structure(source_dir): - pdf_path = source_dir / "test_plot.pdf" - pdf_path.write_text("%PDF-1.4\nTest plot content") - metadata_path = source_dir / "metadata.yaml" - metadata_path.write_text("title: Container Test\nauthor: CI Pipeline\n") - - -def test_build_gallery(tmpdir): - sys.path.insert(0, '.') - from gallery import build_gallery, GalleryConfig - from unittest.mock import patch - with tempfile.TemporaryDirectory() as temp_dir: - temp_path = Path(temp_dir) - source_dir = temp_path / "source" - web_dir = temp_path / "web" - source_dir.mkdir() - web_dir.mkdir() - create_test_structure(source_dir) - - # Mock the PDF conversion and create the expected PNG file - def mock_convert_pdf_to_png(pdf_path): - png_path = pdf_path.with_suffix('.png') - png_path.write_text("Mock PNG content") - - with patch('gallery.convert_pdf_to_png', side_effect=mock_convert_pdf_to_png): - try: - build_gallery(GalleryConfig(tmpdir), source_dir=source_dir, web_dir=web_dir) - html_file = web_dir / "index.html" - assert html_file.exists() - pdf_file = web_dir / "test_plot.pdf" - assert pdf_file.exists() - png_file = web_dir / "test_plot.png" - assert png_file.exists() - except Exception as e: - pytest.skip(f"Gallery generation failed: {e}") diff --git a/tests/test_generate_gallery.py b/tests/test_generate_gallery.py index 38ec3fc..54a5d42 100644 --- a/tests/test_generate_gallery.py +++ b/tests/test_generate_gallery.py @@ -1,17 +1,17 @@ import os +from datetime import datetime from pathlib import Path -from unittest.mock import patch, MagicMock -import pytest +from unittest.mock import patch + from gallery import ( - convert_pdf_to_png, - needs_update, build_gallery, calculate_directory_stats, - format_file_size, + convert_pdf_to_png, datetime_from_timestamp, - strftime_filter + format_file_size, + needs_update, + strftime_filter, ) -from datetime import datetime def test_format_file_size(): @@ -46,6 +46,7 @@ def test_needs_update_target_newer(tmp_path): # Make target newer by modifying its timestamp import time + time.sleep(0.1) target.touch() @@ -58,9 +59,10 @@ def test_needs_update_source_newer(tmp_path): target.write_text("test") import time + time.sleep(0.1) source.write_text("test") - + # Force different modification times with 31+ second buffer target_time = target.stat().st_mtime source_time = target_time + 40 # 40 seconds newer (> 30 second buffer) @@ -69,28 +71,24 @@ def test_needs_update_source_newer(tmp_path): assert needs_update(source, target) is True -@patch('subprocess.run') -def test_convert_pdf_to_png_success(mock_run, tmp_path): +@patch("gallery.utils.processing._convert_pdf_pymupdf") +def test_convert_pdf_to_png_success(mock_convert, tmp_path): from gallery import GalleryConfig pdf_path = tmp_path / "test.pdf" png_path = tmp_path / "test.png" pdf_path.write_text("fake pdf") - mock_run.return_value = MagicMock(returncode=0) + config = GalleryConfig(tmp_path) + convert_pdf_to_png(pdf_path, config) - convert_pdf_to_png(pdf_path, GalleryConfig(tmp_path)) - - mock_run.assert_called_once() - call_args = mock_run.call_args[0][0] - assert call_args[0] == "convert" - assert str(pdf_path) in call_args - assert str(png_path) in call_args + mock_convert.assert_called_once_with(pdf_path, png_path, config.png_dpi) -@patch('subprocess.run') -def test_convert_pdf_to_png_already_exists_newer(mock_run, tmp_path): +@patch("gallery.utils.processing._convert_pdf_pymupdf") +def test_convert_pdf_to_png_already_exists_newer(mock_convert, tmp_path): from gallery import GalleryConfig + pdf_path = tmp_path / "test.pdf" png_path = tmp_path / "test.png" @@ -104,27 +102,27 @@ def test_convert_pdf_to_png_already_exists_newer(mock_run, tmp_path): convert_pdf_to_png(pdf_path, GalleryConfig(tmp_path)) - # Should not call subprocess since PNG is newer - mock_run.assert_not_called() + # Should not convert since PNG is newer + mock_convert.assert_not_called() -@patch('subprocess.run') -def test_convert_pdf_to_png_pdf_newer(mock_run, tmp_path): +@patch("gallery.utils.processing._convert_pdf_pymupdf") +def test_convert_pdf_to_png_pdf_newer(mock_convert, tmp_path): from gallery import GalleryConfig + pdf_path = tmp_path / "test.pdf" png_path = tmp_path / "test.png" png_path.write_text("fake png") import time + time.sleep(0.1) pdf_path.write_text("fake pdf") - mock_run.return_value = MagicMock(returncode=0) - convert_pdf_to_png(pdf_path, GalleryConfig(tmp_path)) - # Should call subprocess since PDF is newer - mock_run.assert_called_once() + # Should convert since PDF is newer + mock_convert.assert_called_once() def test_calculate_directory_stats_empty(tmp_path): @@ -181,24 +179,17 @@ def test_strftime_filter(): assert formatted == "2023-09-08 14:30" -@patch('gallery.builder.render_gallery_page') -@patch('gallery.utils.metadata.save_metadata_cache') -@patch('gallery.utils.metadata.resolve_metadata_for_plot') -@patch('gallery.utils.metadata.merge_metadata') -@patch('gallery.utils.metadata.load_folder_metadata') -@patch('gallery.utils.processing.convert_pdf_to_png') -@patch('shutil.copy2') +@patch("gallery.builder.save_metadata_cache") +@patch("gallery.utils.processing.resolve_metadata_for_plot") +@patch("gallery.builder.merge_metadata") +@patch("gallery.builder.load_folder_metadata") +@patch("gallery.utils.processing.convert_pdf_to_png") +@patch("shutil.copy2") def test_build_gallery_basic( - mock_copy, - mock_convert, - mock_load_folder, - mock_merge, - mock_resolve, - mock_save_cache, - mock_template, - tmp_path + mock_copy, mock_convert, mock_load_folder, mock_merge, mock_resolve, mock_save_cache, tmp_path ): - from gallery import GalleryConfig + from gallery import GalleryConfig, get_template + source_dir = tmp_path / "source" web_dir = tmp_path / "web" source_dir.mkdir() @@ -214,29 +205,24 @@ def test_build_gallery_basic( mock_load_folder.return_value = {"folder": "metadata"} mock_merge.return_value = {"merged": "metadata"} mock_resolve.return_value = {"plot": "metadata"} - mock_template.render.return_value = "test" - build_gallery(GalleryConfig(tmp_path), source_dir, web_dir) + config = GalleryConfig(tmp_path) + build_gallery(config, source_dir, web_dir, template=get_template()) # Verify mocks were called - mock_convert.assert_called_once_with(pdf_file) + mock_convert.assert_called_once_with(pdf_file, config=config) mock_copy.assert_called() # Should be called for PDF mock_save_cache.assert_called_once() - # Check HTML file was created + # Check HTML file was created by the real render_gallery_page html_file = web_dir / "index.html" assert html_file.exists() -@patch('gallery.builder.render_gallery_page') -@patch('gallery.utils.metadata.save_metadata_cache') -@patch('gallery.utils.metadata.load_folder_metadata') -def test_build_gallery_with_subdirs( - mock_load_folder, - mock_save_cache, - mock_template, - tmp_path -): +@patch("gallery.builder.render_gallery_page") +@patch("gallery.builder.save_metadata_cache") +@patch("gallery.builder.load_folder_metadata") +def test_build_gallery_with_subdirs(mock_load_folder, mock_save_cache, mock_template, tmp_path): source_dir = tmp_path / "source" web_dir = tmp_path / "web" source_dir.mkdir() @@ -248,8 +234,9 @@ def test_build_gallery_with_subdirs( mock_load_folder.return_value = {} mock_template.render.return_value = "test" - + from gallery import GalleryConfig + build_gallery(config=GalleryConfig(tmp_path), source_dir=source_dir, web_dir=web_dir) # Check subdirectory was created in web @@ -258,8 +245,8 @@ def test_build_gallery_with_subdirs( assert web_subdir.is_dir() -@patch('gallery.utils.processing.needs_update') -@patch('shutil.copy2') +@patch("gallery.utils.processing.needs_update") +@patch("shutil.copy2") def test_build_gallery_skip_up_to_date(mock_copy, mock_needs_update, tmp_path): source_dir = tmp_path / "source" web_dir = tmp_path / "web" @@ -282,8 +269,10 @@ def test_build_gallery_skip_up_to_date(mock_copy, mock_needs_update, tmp_path): mock_needs_update.return_value = False from gallery import get_template + get_template() from gallery import GalleryConfig + build_gallery(GalleryConfig(tmp_path), source_dir, web_dir) # copy2 should not be called since files are up to date diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 5dc8a70..8d95515 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -1,79 +1,81 @@ import json + import pytest -from utils import metadata import yaml +from gallery.utils import metadata + def test_load_metadata_file_yaml(tmp_path): - data = {'a': 1, 'b': 'test'} - yaml_path = tmp_path / 'meta.yaml' + data = {"a": 1, "b": "test"} + yaml_path = tmp_path / "meta.yaml" yaml_path.write_text(yaml.dump(data)) result = metadata.load_metadata_file(yaml_path) assert result == data def test_load_metadata_file_json(tmp_path): - data = {'x': 42, 'y': 'hello'} - json_path = tmp_path / 'meta.json' + data = {"x": 42, "y": "hello"} + json_path = tmp_path / "meta.json" json_path.write_text(json.dumps(data)) result = metadata.load_metadata_file(json_path) assert result == data def test_load_metadata_file_missing(tmp_path): - missing_path = tmp_path / 'nope.yaml' + missing_path = tmp_path / "nope.yaml" result = metadata.load_metadata_file(missing_path) assert result == {} def test_load_metadata_file_yml_extension(tmp_path): - data = {'test': 'yml_format'} - yml_path = tmp_path / 'meta.yml' + data = {"test": "yml_format"} + yml_path = tmp_path / "meta.yml" yml_path.write_text(yaml.dump(data)) result = metadata.load_metadata_file(yml_path) assert result == data def test_load_metadata_file_unknown_format(tmp_path): - txt_path = tmp_path / 'meta.txt' - txt_path.write_text('some text') + txt_path = tmp_path / "meta.txt" + txt_path.write_text("some text") result = metadata.load_metadata_file(txt_path) assert result == {} def test_load_metadata_file_malformed_yaml(tmp_path): - yaml_path = tmp_path / 'bad.yaml' - yaml_path.write_text('invalid: yaml: content: [') + yaml_path = tmp_path / "bad.yaml" + yaml_path.write_text("invalid: yaml: content: [") with pytest.raises(yaml.YAMLError): metadata.load_metadata_file(yaml_path) def test_load_metadata_file_malformed_json(tmp_path): - json_path = tmp_path / 'bad.json' + json_path = tmp_path / "bad.json" json_path.write_text('{"invalid": json}') with pytest.raises(json.JSONDecodeError): metadata.load_metadata_file(json_path) def test_load_folder_metadata_yaml(tmp_path): - data = {'folder': 'metadata'} - metadata_path = tmp_path / 'metadata.yaml' + data = {"folder": "metadata"} + metadata_path = tmp_path / "metadata.yaml" metadata_path.write_text(yaml.dump(data)) result = metadata.load_folder_metadata(tmp_path) assert result == data def test_load_folder_metadata_yml(tmp_path): - data = {'folder': 'metadata_yml'} - metadata_path = tmp_path / 'metadata.yml' + data = {"folder": "metadata_yml"} + metadata_path = tmp_path / "metadata.yml" metadata_path.write_text(yaml.dump(data)) result = metadata.load_folder_metadata(tmp_path) assert result == data def test_load_folder_metadata_json(tmp_path): - data = {'folder': 'metadata_json'} - metadata_path = tmp_path / 'metadata.json' + data = {"folder": "metadata_json"} + metadata_path = tmp_path / "metadata.json" metadata_path.write_text(json.dumps(data)) result = metadata.load_folder_metadata(tmp_path) assert result == data @@ -85,21 +87,21 @@ def test_load_folder_metadata_missing(tmp_path): def test_get_metadata_file_path_existing_yaml(tmp_path): - metadata_path = tmp_path / 'metadata.yaml' - metadata_path.write_text('test: data') + metadata_path = tmp_path / "metadata.yaml" + metadata_path.write_text("test: data") result = metadata.get_metadata_file_path(tmp_path) assert result == str(metadata_path) def test_get_metadata_file_path_existing_yml(tmp_path): - metadata_path = tmp_path / 'metadata.yml' - metadata_path.write_text('test: data') + metadata_path = tmp_path / "metadata.yml" + metadata_path.write_text("test: data") result = metadata.get_metadata_file_path(tmp_path) assert result == str(metadata_path) def test_get_metadata_file_path_existing_json(tmp_path): - metadata_path = tmp_path / 'metadata.json' + metadata_path = tmp_path / "metadata.json" metadata_path.write_text('{"test": "data"}') result = metadata.get_metadata_file_path(tmp_path) assert result == str(metadata_path) @@ -107,28 +109,27 @@ def test_get_metadata_file_path_existing_json(tmp_path): def test_get_metadata_file_path_none_existing(tmp_path): result = metadata.get_metadata_file_path(tmp_path) - expected = str(tmp_path / 'metadata.yaml') + expected = str(tmp_path / "metadata.yaml") assert result == expected def test_merge_metadata(): - parent = {'project': 'Test', 'version': '1.0', 'author': 'Parent'} - child = {'experiment': 'A', 'version': '1.1'} + parent = {"project": "Test", "version": "1.0", "author": "Parent"} + child = {"experiment": "A", "version": "1.1"} merged = metadata.merge_metadata(parent, child) - expected = {'project': 'Test', 'version': '1.1', - 'author': 'Parent', 'experiment': 'A'} + expected = {"project": "Test", "version": "1.1", "author": "Parent", "experiment": "A"} assert merged == expected def test_merge_metadata_empty_parent(): parent = {} - child = {'experiment': 'A', 'version': '1.1'} + child = {"experiment": "A", "version": "1.1"} merged = metadata.merge_metadata(parent, child) assert merged == child def test_merge_metadata_empty_child(): - parent = {'project': 'Test', 'version': '1.0'} + parent = {"project": "Test", "version": "1.0"} child = {} merged = metadata.merge_metadata(parent, child) assert merged == parent @@ -136,35 +137,34 @@ def test_merge_metadata_empty_child(): def test_resolve_metadata_for_plot_with_specific_yaml(tmp_path): # Create plot-specific metadata file - plot_path = tmp_path / 'test_plot.pdf' - plot_metadata_path = tmp_path / 'test_plot.yaml' - plot_metadata = {'specific': 'plot_data', 'override': 'plot_value'} + plot_path = tmp_path / "test_plot.pdf" + plot_metadata_path = tmp_path / "test_plot.yaml" + plot_metadata = {"specific": "plot_data", "override": "plot_value"} plot_metadata_path.write_text(yaml.dump(plot_metadata)) - inherited = {'general': 'data', 'override': 'inherited_value'} + inherited = {"general": "data", "override": "inherited_value"} result = metadata.resolve_metadata_for_plot(plot_path, inherited) - expected = {'general': 'data', - 'override': 'plot_value', 'specific': 'plot_data'} + expected = {"general": "data", "override": "plot_value", "specific": "plot_data"} assert result == expected def test_resolve_metadata_for_plot_with_specific_json(tmp_path): - plot_path = tmp_path / 'test_plot.pdf' - plot_metadata_path = tmp_path / 'test_plot.json' - plot_metadata = {'specific': 'plot_data_json'} + plot_path = tmp_path / "test_plot.pdf" + plot_metadata_path = tmp_path / "test_plot.json" + plot_metadata = {"specific": "plot_data_json"} plot_metadata_path.write_text(json.dumps(plot_metadata)) - inherited = {'general': 'data'} + inherited = {"general": "data"} result = metadata.resolve_metadata_for_plot(plot_path, inherited) - expected = {'general': 'data', 'specific': 'plot_data_json'} + expected = {"general": "data", "specific": "plot_data_json"} assert result == expected def test_resolve_metadata_for_plot_no_specific(tmp_path): - plot_path = tmp_path / 'test_plot.pdf' - inherited = {'general': 'data', 'inherited': 'value'} + plot_path = tmp_path / "test_plot.pdf" + inherited = {"general": "data", "inherited": "value"} result = metadata.resolve_metadata_for_plot(plot_path, inherited) # Should return copy of inherited metadata @@ -173,17 +173,14 @@ def test_resolve_metadata_for_plot_no_specific(tmp_path): def test_save_metadata_cache(tmp_path): - cache_data = { - 'plot1': {'title': 'Plot 1', 'author': 'Test'}, - 'plot2': {'title': 'Plot 2', 'experiment': 'B'} - } + cache_data = {"plot1": {"title": "Plot 1", "author": "Test"}, "plot2": {"title": "Plot 2", "experiment": "B"}} metadata.save_metadata_cache(tmp_path, cache_data) - cache_file = tmp_path / 'meta_cache.json' + cache_file = tmp_path / "meta_cache.json" assert cache_file.exists() - with cache_file.open('r') as f: + with cache_file.open("r") as f: loaded_data = json.load(f) assert loaded_data == cache_data @@ -193,10 +190,10 @@ def test_save_metadata_cache_empty(tmp_path): cache_data = {} metadata.save_metadata_cache(tmp_path, cache_data) - cache_file = tmp_path / 'meta_cache.json' + cache_file = tmp_path / "meta_cache.json" assert cache_file.exists() - with cache_file.open('r') as f: + with cache_file.open("r") as f: loaded_data = json.load(f) assert loaded_data == {} diff --git a/tests/validate_metadata.py b/tests/validate_metadata.py index 3f21ebe..ab87c6a 100644 --- a/tests/validate_metadata.py +++ b/tests/validate_metadata.py @@ -6,86 +6,87 @@ This script validates metadata files in the gallery source directories, checking for proper YAML/JSON syntax and common field validation. """ -import sys import json -import yaml +import sys from pathlib import Path -from typing import Dict, Any, List +from typing import List + +import yaml def validate_metadata_file(file_path: Path) -> tuple[bool, List[str]]: """ Validate a single metadata file. - + Args: file_path: Path to the metadata file - + Returns: Tuple of (is_valid, error_messages) """ errors = [] - + if not file_path.exists(): errors.append(f"File does not exist: {file_path}") return False, errors - + try: - with file_path.open('r', encoding='utf-8') as f: + with file_path.open("r", encoding="utf-8") as f: suffix_lower = file_path.suffix.lower() - if suffix_lower in ['.yaml', '.yml']: + if suffix_lower in [".yaml", ".yml"]: data = yaml.safe_load(f) - elif suffix_lower == '.json': + elif suffix_lower == ".json": data = json.load(f) else: errors.append(f"Unsupported file format: {file_path}") return False, errors - + if data is None: errors.append(f"Empty metadata file: {file_path}") return False, errors - + # Basic validation if not isinstance(data, dict): errors.append(f"Metadata must be a dictionary: {file_path}") return False, errors - + # Check for common issues - if 'title' in data and not isinstance(data['title'], str): + if "title" in data and not isinstance(data["title"], str): errors.append(f"Title must be a string: {file_path}") - - if 'tags' in data and not isinstance(data['tags'], list): + + if "tags" in data and not isinstance(data["tags"], list): errors.append(f"Tags must be a list: {file_path}") - - if 'author' in data and not isinstance(data['author'], dict): + + if "author" in data and not isinstance(data["author"], dict): errors.append(f"Author must be a dictionary: {file_path}") - + except (yaml.YAMLError, json.JSONDecodeError) as e: errors.append(f"Parse error in {file_path}: {e}") return False, errors except Exception as e: errors.append(f"Unexpected error reading {file_path}: {e}") return False, errors - + return len(errors) == 0, errors def find_metadata_files(root_dir: Path) -> List[Path]: """ Find all metadata files in a directory tree. - + Args: root_dir: Root directory to search - + Returns: List of metadata file paths """ metadata_files = [] - - for pattern in ['**/*.yaml', '**/*.yml', '**/*.json']: + + for pattern in ["**/*.yaml", "**/*.yml", "**/*.json"]: for file_path in root_dir.glob(pattern): - if file_path.name.startswith('meta.') or file_path.stem != file_path.name: + if file_path.name.startswith("meta.") or file_path.stem != file_path.name: metadata_files.append(file_path) - + return metadata_files @@ -94,32 +95,32 @@ def main(): if len(sys.argv) != 2: print("Usage: python validate_metadata.py ") sys.exit(1) - + root_dir = Path(sys.argv[1]) - + if not root_dir.exists(): print(f"Error: Directory does not exist: {root_dir}") sys.exit(1) - + if not root_dir.is_dir(): print(f"Error: Not a directory: {root_dir}") sys.exit(1) - + print(f"Validating metadata files in: {root_dir}") print("-" * 50) - + metadata_files = find_metadata_files(root_dir) - + if not metadata_files: print("No metadata files found.") return - + total_files = len(metadata_files) valid_files = 0 - + for file_path in metadata_files: is_valid, errors = validate_metadata_file(file_path) - + if is_valid: print(f"✓ {file_path.relative_to(root_dir)}") valid_files += 1 @@ -127,10 +128,10 @@ def main(): print(f"✗ {file_path.relative_to(root_dir)}") for error in errors: print(f" - {error}") - + print("-" * 50) print(f"Summary: {valid_files}/{total_files} files valid") - + if valid_files != total_files: sys.exit(1)