From bda332ddd935e6b6f62200111863f35abafe0cab Mon Sep 17 00:00:00 2001 From: dippatel1994 Date: Thu, 11 Jun 2026 16:15:59 -0400 Subject: [PATCH 1/2] feat: user-suppliable venue style packs A venue is now a directory convention: {methodology_style_guide.md, plot_style_guide.md} plus an optional venue.yaml (display_name, aspect_ratio default, font preferences). --venue resolves built-in packs first (data/guidelines//, flat files for neurips), then user packs from ~/.config/paperbanana/venues (overridable via PAPERBANANA_VENUE_DIR or --venue-dir). Unknown venues fail fast with the available packs from both sources instead of silently falling back to neurips. - new resolver module paperbanana/guidelines/venues.py: list_venues(), resolve_venue() -> VenuePack, validate_venue(), venue.yaml parsing (yaml.safe_load, all fields optional), select_aspect_ratio() - guideline loaders accept venue_dir and resolve through the venue pack registry; Settings.venue is now an open name with a venue_dir companion setting (pipeline.venue_dir / PAPERBANANA_VENUE_DIR) - venue.yaml aspect_ratio becomes the default ratio when the CLI did not pass one (user > venue > planner); fonts are appended as a note to the loaded style guides - new CLI commands: 'paperbanana venues list' (built-in + user with source column) and 'paperbanana venues init ' (scaffolds a pack from the NeurIPS templates + commented venue.yaml) - replaced hardcoded venue tuples in cli.py, workflow_runner.py and the studio with resolver-backed validation; studio dropdown now lists user packs - README: Custom venue style packs section; tests for precedence (built-in beats user), error listing, venue.yaml parsing, init scaffolding, and aspect-ratio defaulting Fixes #89 --- README.md | 32 ++- configs/config.yaml | 4 +- paperbanana/cli.py | 248 +++++++++++++--- paperbanana/core/config.py | 23 +- paperbanana/core/pipeline.py | 34 ++- paperbanana/core/workflow_runner.py | 10 +- paperbanana/guidelines/methodology.py | 47 ++- paperbanana/guidelines/plots.py | 47 ++- paperbanana/guidelines/venues.py | 309 ++++++++++++++++++++ paperbanana/studio/app.py | 9 +- paperbanana/studio/runner.py | 4 +- tests/test_venue.py | 35 ++- tests/test_venue_packs.py | 399 ++++++++++++++++++++++++++ 13 files changed, 1097 insertions(+), 104 deletions(-) create mode 100644 paperbanana/guidelines/venues.py create mode 100644 tests/test_venue_packs.py diff --git a/README.md b/README.md index a7f6d977..f2151518 100644 --- a/README.md +++ b/README.md @@ -299,6 +299,36 @@ paperbanana plot \ Plots are rendered via VLM-generated matplotlib code — no image-generation provider or credentials are required. +### `paperbanana venues` -- Custom Venue Style Packs + +`--venue` selects a *venue style pack*: a directory with `methodology_style_guide.md`, `plot_style_guide.md`, and an optional `venue.yaml`. Built-in packs (`neurips`, `icml`, `acl`, `ieee`) ship with PaperBanana; you can add your own under `~/.config/paperbanana/venues/` (override with `--venue-dir` or `PAPERBANANA_VENUE_DIR`) without touching the repo: + +```bash +# 1. Scaffold a pack (seeds both guides from the NeurIPS templates) +paperbanana venues init mylab + +# 2. Edit the style guides — or generate them from a corpus of example figures: +# paperbanana guidelines synthesize --reference-set ./examples \ +# --output ~/.config/paperbanana/venues/mylab/methodology_style_guide.md + +# 3. Use it anywhere --venue is accepted +paperbanana generate --input method.txt --caption "Overview" --venue mylab + +# See everything that's available (built-in + user, with source) +paperbanana venues list +``` + +`venue.yaml` (all fields optional): + +```yaml +display_name: "My Lab Style" # shown by `paperbanana venues list` +aspect_ratio: "16:9" # default --aspect-ratio for this venue's runs +fonts: # preferred fonts, appended to the style guides + - "Helvetica" +``` + +On a name clash, built-in packs win — user packs cannot shadow built-in venues. Unknown venue names fail fast with the list of available packs from both sources. + ### `paperbanana batch` -- Batch Generation Generate multiple methodology diagrams from a single manifest file (YAML or JSON). Each item runs the full pipeline; outputs are written under `outputs/batch_/run_/` and a `batch_report.json` summarizes all runs. @@ -418,7 +448,7 @@ Paths are resolved relative to the manifest file’s directory. | `--optimize` | | Input optimization per item | | `--format` | `-f` | png, jpeg, or webp | | `--save-prompts` / `--no-save-prompts` | | Persist prompts (default: on, same as `plot`) | -| `--venue` | | Venue style (neurips, icml, acl, ieee, custom) | +| `--venue` | | Venue style pack: built-in (neurips, icml, acl, ieee), a user pack, or `custom` | | `--aspect-ratio` | `-ar` | Default aspect ratio when not set in the manifest | | `--verbose` | `-v` | Verbose logging | diff --git a/configs/config.yaml b/configs/config.yaml index f990068c..4e70d628 100644 --- a/configs/config.yaml +++ b/configs/config.yaml @@ -20,7 +20,9 @@ pipeline: # optimize_inputs: true # Preprocess inputs for better generation output_resolution: "2k" # 1k, 2k, 4k diagram_type: methodology # methodology, statistical_plot - # venue: neurips # Target venue style: neurips, icml, acl, ieee, custom + # venue: neurips # Venue style pack: built-in (neurips, icml, acl, ieee), + # # a user pack name (see `paperbanana venues list`), or custom + # venue_dir: ~/.config/paperbanana/venues # User venue pack directory override # Reference set reference: diff --git a/paperbanana/cli.py b/paperbanana/cli.py index f824ada5..7bce66fd 100644 --- a/paperbanana/cli.py +++ b/paperbanana/cli.py @@ -96,6 +96,25 @@ def _main( ) app.add_typer(guidelines_app, name="guidelines") +# ── Venues subcommand group ─────────────────────────────────────── +venues_app = typer.Typer( + name="venues", + help="Manage venue style packs (built-in and user-supplied).", + no_args_is_help=True, +) +app.add_typer(venues_app, name="venues") + + +def _validate_venue_or_exit(venue: Optional[str], venue_dir: Optional[str] = None) -> None: + """Validate a --venue value against built-in and user style packs.""" + from paperbanana.guidelines.venues import UnknownVenueError, validate_venue + + try: + validate_venue(venue, extra_dir=venue_dir) + except UnknownVenueError as e: + console.print(f"[red]Error: {e}[/red]") + raise typer.Exit(1) + def _require_pdf_dep() -> None: """Raise a clean error if PyMuPDF is not installed.""" @@ -385,7 +404,18 @@ def generate( venue: Optional[str] = typer.Option( None, "--venue", - help="Target venue style (neurips, icml, acl, ieee, custom)", + help=( + "Venue style pack: built-in (neurips, icml, acl, ieee), a user pack " + "(see 'paperbanana venues list'), or 'custom' (flat guideline files)" + ), + ), + venue_dir: Optional[str] = typer.Option( + None, + "--venue-dir", + help=( + "User venue style pack directory " + "(default: $PAPERBANANA_VENUE_DIR or ~/.config/paperbanana/venues)" + ), ), export_tikz: bool = typer.Option( False, @@ -442,11 +472,7 @@ def generate( "[red]Error: --exemplar-mode must be external_then_rerank or external_only[/red]" ) raise typer.Exit(1) - if venue and venue.lower() not in ("neurips", "icml", "acl", "ieee", "custom"): - console.print( - f"[red]Error: --venue must be neurips, icml, acl, ieee, or custom. Got: {venue}[/red]" - ) - raise typer.Exit(1) + _validate_venue_or_exit(venue, venue_dir) if vector_export and vector_export.lower() not in ("none", "svg", "pdf", "both"): console.print("[red]Error: --vector-export must be none, svg, pdf, or both[/red]") raise typer.Exit(1) @@ -529,6 +555,8 @@ def generate( overrides["num_candidates"] = num_candidates if venue: overrides["venue"] = venue + if venue_dir: + overrides["venue_dir"] = venue_dir if vector_export is not None: overrides["vector_export"] = vector_export.lower() if prompt_dir: @@ -1580,7 +1608,10 @@ def batch( venue: Optional[str] = typer.Option( None, "--venue", - help="Target venue style (neurips, icml, acl, ieee, custom)", + help=( + "Venue style pack: built-in (neurips, icml, acl, ieee), a user pack " + "(see 'paperbanana venues list'), or 'custom' (flat guideline files)" + ), ), auto_download_data: bool = typer.Option( False, @@ -1606,11 +1637,7 @@ def batch( if format not in ("png", "jpeg", "webp"): console.print(f"[red]Error: Format must be png, jpeg, or webp. Got: {format}[/red]") raise typer.Exit(1) - if venue and venue.lower() not in ("neurips", "icml", "acl", "ieee", "custom"): - console.print( - f"[red]Error: --venue must be neurips, icml, acl, ieee, or custom. Got: {venue}[/red]" - ) - raise typer.Exit(1) + _validate_venue_or_exit(venue) if max_retries < 0: console.print("[red]Error: --max-retries must be >= 0[/red]") raise typer.Exit(1) @@ -1942,7 +1969,10 @@ def orchestrate( venue: Optional[str] = typer.Option( None, "--venue", - help="Target venue style (neurips, icml, acl, ieee, custom)", + help=( + "Venue style pack: built-in (neurips, icml, acl, ieee), a user pack " + "(see 'paperbanana venues list'), or 'custom' (flat guideline files)" + ), ), concurrency: int = typer.Option( 1, "--concurrency", help="Maximum concurrent figure generations" @@ -1959,11 +1989,7 @@ def orchestrate( if format not in ("png", "jpeg", "webp"): console.print(f"[red]Error: Format must be png, jpeg, or webp. Got: {format}[/red]") raise typer.Exit(1) - if venue and venue.lower() not in ("neurips", "icml", "acl", "ieee", "custom"): - console.print( - f"[red]Error: --venue must be neurips, icml, acl, ieee, or custom. Got: {venue}[/red]" - ) - raise typer.Exit(1) + _validate_venue_or_exit(venue) if max_method_figures < 1: console.print("[red]Error: --max-method-figures must be >= 1[/red]") raise typer.Exit(1) @@ -2172,7 +2198,10 @@ def plot_batch( venue: Optional[str] = typer.Option( None, "--venue", - help="Target venue style (neurips, icml, acl, ieee, custom)", + help=( + "Venue style pack: built-in (neurips, icml, acl, ieee), a user pack " + "(see 'paperbanana venues list'), or 'custom' (flat guideline files)" + ), ), aspect_ratio: Optional[str] = typer.Option( None, @@ -2196,11 +2225,7 @@ def plot_batch( if format not in ("png", "jpeg", "webp"): console.print(f"[red]Error: Format must be png, jpeg, or webp. Got: {format}[/red]") raise typer.Exit(1) - if venue and venue.lower() not in ("neurips", "icml", "acl", "ieee", "custom"): - console.print( - f"[red]Error: --venue must be neurips, icml, acl, ieee, or custom. Got: {venue}[/red]" - ) - raise typer.Exit(1) + _validate_venue_or_exit(venue) if max_retries < 0: console.print("[red]Error: --max-retries must be >= 0[/red]") raise typer.Exit(1) @@ -2330,7 +2355,18 @@ def plot( venue: Optional[str] = typer.Option( None, "--venue", - help="Target venue style (neurips, icml, acl, ieee, custom)", + help=( + "Venue style pack: built-in (neurips, icml, acl, ieee), a user pack " + "(see 'paperbanana venues list'), or 'custom' (flat guideline files)" + ), + ), + venue_dir: Optional[str] = typer.Option( + None, + "--venue-dir", + help=( + "User venue style pack directory " + "(default: $PAPERBANANA_VENUE_DIR or ~/.config/paperbanana/venues)" + ), ), cost_only: bool = typer.Option( False, @@ -2366,11 +2402,7 @@ def plot( if format not in ("png", "jpeg", "webp"): console.print(f"[red]Error: Format must be png, jpeg, or webp. Got: {format}[/red]") raise typer.Exit(1) - if venue and venue.lower() not in ("neurips", "icml", "acl", "ieee", "custom"): - console.print( - f"[red]Error: --venue must be neurips, icml, acl, ieee, or custom. Got: {venue}[/red]" - ) - raise typer.Exit(1) + _validate_venue_or_exit(venue, venue_dir) configure_logging(verbose=verbose) data_path = Path(data) @@ -2410,6 +2442,8 @@ def plot( overrides["output_format"] = format if venue: overrides["venue"] = venue + if venue_dir: + overrides["venue_dir"] = venue_dir if budget is not None: overrides["budget_usd"] = budget if generate_caption: @@ -2540,7 +2574,10 @@ def tikz( venue: Optional[str] = typer.Option( None, "--venue", - help="Target venue style (neurips, icml, acl, ieee, custom)", + help=( + "Venue style pack: built-in (neurips, icml, acl, ieee), a user pack " + "(see 'paperbanana venues list'), or 'custom' (flat guideline files)" + ), ), config: Optional[str] = typer.Option(None, "--config", help="Path to a YAML config file"), verbose: bool = typer.Option(False, "--verbose", "-v", help="Show detailed progress"), @@ -2555,11 +2592,7 @@ def tikz( console.print("[red]Error: --diagram-type must be 'diagram' or 'plot'[/red]") raise typer.Exit(1) - if venue and venue.lower() not in ("neurips", "icml", "acl", "ieee", "custom"): - console.print( - f"[red]Error: --venue must be neurips, icml, acl, ieee, or custom. Got: {venue}[/red]" - ) - raise typer.Exit(1) + _validate_venue_or_exit(venue) configure_logging(verbose=verbose) @@ -4105,6 +4138,149 @@ async def _run() -> str: console.print(f" Cost: [bold]${cost_tracker.total_cost:.4f}[/bold]") +# ── venues subcommands ───────────────────────────────────────────── + + +_VENUE_YAML_TEMPLATE = """\ +# PaperBanana venue style pack configuration. +# All fields are optional — delete or comment out anything you don't need. + +# Human-readable name shown by `paperbanana venues list`. +display_name: "{display_name}" + +# Default --aspect-ratio for runs with this venue (used when the CLI flag +# is not passed). One of: 1:1, 2:3, 3:2, 3:4, 4:3, 9:16, 16:9, 21:9. +# aspect_ratio: "16:9" + +# Preferred font families, appended as a note to the style guides. +# fonts: +# - "Helvetica" +# - "Arial" +""" + + +@venues_app.command(name="list") +def venues_list( + venue_dir: Optional[str] = typer.Option( + None, + "--venue-dir", + help=( + "User venue style pack directory " + "(default: $PAPERBANANA_VENUE_DIR or ~/.config/paperbanana/venues)" + ), + ), +): + """List available venue style packs (built-in and user).""" + from paperbanana.guidelines.venues import ( + list_venues, + load_venue_config, + resolve_user_venue_dir, + ) + + venues = list_venues(extra_dir=venue_dir) + + table = Table(title="Venue Style Packs") + table.add_column("Name", style="bold") + table.add_column("Source") + table.add_column("Display Name") + table.add_column("Aspect Ratio") + table.add_column("Path", overflow="fold") + + for info in venues.values(): + try: + config = load_venue_config(info.dir) + except (ValueError, OSError): + config = None + table.add_row( + info.name, + info.source, + (config.display_name if config and config.display_name else "—"), + (config.aspect_ratio if config and config.aspect_ratio else "—"), + str(info.dir), + ) + + console.print(table) + console.print( + f"\nUser venue directory: [bold]{resolve_user_venue_dir(venue_dir)}[/bold]\n" + "Scaffold a new pack with: [bold]paperbanana venues init [/bold]" + ) + + +@venues_app.command(name="init") +def venues_init( + name: str = typer.Argument(..., help="Name for the new venue pack (used with --venue)"), + venue_dir: Optional[str] = typer.Option( + None, + "--venue-dir", + help=( + "User venue style pack directory " + "(default: $PAPERBANANA_VENUE_DIR or ~/.config/paperbanana/venues)" + ), + ), +): + """Scaffold a user venue style pack from the NeurIPS templates.""" + from paperbanana.guidelines.methodology import load_methodology_guidelines + from paperbanana.guidelines.plots import load_plot_guidelines + from paperbanana.guidelines.venues import ( + DEFAULT_BUILTIN_GUIDELINES_DIR, + METHODOLOGY_GUIDE_FILENAME, + PLOT_GUIDE_FILENAME, + RESERVED_VENUE_NAMES, + VENUE_CONFIG_FILENAME, + list_venues, + resolve_user_venue_dir, + ) + + venue_name = name.strip().lower() + if not venue_name or not all(c.isalnum() or c in ("-", "_") for c in venue_name): + console.print( + f"[red]Error: Invalid venue name '{name}'. " + "Use letters, digits, hyphens, and underscores only.[/red]" + ) + raise typer.Exit(1) + if venue_name in RESERVED_VENUE_NAMES: + console.print(f"[red]Error: Venue name '{venue_name}' is reserved.[/red]") + raise typer.Exit(1) + + existing = list_venues(extra_dir=venue_dir) + if venue_name in existing: + info = existing[venue_name] + console.print( + f"[red]Error: Venue '{venue_name}' already exists " + f"({info.source}: {info.dir}).[/red]\n" + "Built-in venues cannot be shadowed; pick a different name." + ) + raise typer.Exit(1) + + target = resolve_user_venue_dir(venue_dir) / venue_name + try: + target.mkdir(parents=True, exist_ok=False) + # Seed both guides from the NeurIPS templates as a starting point. + methodology = load_methodology_guidelines(DEFAULT_BUILTIN_GUIDELINES_DIR, venue="neurips") + plot = load_plot_guidelines(DEFAULT_BUILTIN_GUIDELINES_DIR, venue="neurips") + (target / METHODOLOGY_GUIDE_FILENAME).write_text(methodology, encoding="utf-8") + (target / PLOT_GUIDE_FILENAME).write_text(plot, encoding="utf-8") + (target / VENUE_CONFIG_FILENAME).write_text( + _VENUE_YAML_TEMPLATE.format(display_name=venue_name.upper()), + encoding="utf-8", + ) + except OSError as e: + console.print(f"[red]Error: Cannot create venue pack at {target} ({e}).[/red]") + raise typer.Exit(1) + + console.print( + f"[green]Created venue pack:[/green] [bold]{target}[/bold]\n\n" + f" {METHODOLOGY_GUIDE_FILENAME} — methodology diagram style guide\n" + f" {PLOT_GUIDE_FILENAME} — statistical plot style guide\n" + f" {VENUE_CONFIG_FILENAME} — optional venue metadata\n\n" + "Next steps:\n" + f" 1. Edit the two style guides (or generate them from example figures " + f"with 'paperbanana guidelines synthesize --output {target}/...').\n" + f" 2. Adjust {VENUE_CONFIG_FILENAME} (display name, aspect ratio, fonts).\n" + f" 3. Use it: [bold]paperbanana generate --venue {venue_name} ...[/bold]" + ) + + @app.command() def studio( host: str = typer.Option( diff --git a/paperbanana/core/config.py b/paperbanana/core/config.py index 5d369c79..0d22c9e0 100644 --- a/paperbanana/core/config.py +++ b/paperbanana/core/config.py @@ -12,7 +12,10 @@ OutputFormat = Literal["png", "jpeg", "webp"] ImageQuality = Literal["low", "medium", "high", "auto"] ExemplarRetrievalMode = Literal["external_only", "external_then_rerank"] -Venue = Literal["neurips", "icml", "acl", "ieee", "custom"] +# Venue is an open name resolved against built-in and user style packs at +# pipeline startup (see paperbanana.guidelines.venues). Kept as an alias for +# backward compatibility with earlier Literal-based typing. +Venue = str VectorExportMode = Literal["none", "svg", "pdf", "both"] @@ -81,6 +84,11 @@ class Settings(BaseSettings): exemplar_retrieval_timeout_seconds: float = 20.0 exemplar_retrieval_max_retries: int = 2 venue: Venue = "neurips" + venue_dir: Optional[str] = Field( + default=None, + alias="PAPERBANANA_VENUE_DIR", + description="User venue style pack directory (default: ~/.config/paperbanana/venues)", + ) vector_export: VectorExportMode = "none" num_candidates: int = Field( default=1, @@ -276,13 +284,15 @@ def validate_vector_export(cls, v: Any) -> str: @field_validator("venue", mode="before") @classmethod def validate_venue(cls, v: Any) -> str: - """Validate venue is a supported venue name (case-insensitive).""" + """Normalize the venue name (case-insensitive). + + Venue names are open: they resolve against built-in and user style + packs when the pipeline loads guidelines. Unknown names raise + UnknownVenueError (listing available venues) at that point. + """ if v is None: return "neurips" - v = str(v).lower() - if v not in ("neurips", "icml", "acl", "ieee", "custom"): - raise ValueError(f"venue must be neurips, icml, acl, ieee, or custom. Got: {v}") - return v + return str(v).strip().lower() @classmethod def from_yaml(cls, config_path: str | Path, **overrides: Any) -> Settings: @@ -326,6 +336,7 @@ def _flatten_yaml(config: dict, prefix: str = "") -> dict: "reference.category": "reference_category", "reference.guidelines_path": "guidelines_path", "pipeline.venue": "venue", + "pipeline.venue_dir": "venue_dir", "pipeline.vector_export": "vector_export", "output.dir": "output_dir", "output.format": "output_format", diff --git a/paperbanana/core/pipeline.py b/paperbanana/core/pipeline.py index c2997a8e..f3caa607 100644 --- a/paperbanana/core/pipeline.py +++ b/paperbanana/core/pipeline.py @@ -52,6 +52,7 @@ ) from paperbanana.guidelines.methodology import load_methodology_guidelines from paperbanana.guidelines.plots import load_plot_guidelines +from paperbanana.guidelines.venues import VenuePack, resolve_venue, select_aspect_ratio from paperbanana.providers.registry import ProviderRegistry from paperbanana.reference.exemplar_retrieval import ( ExemplarRetrievalError, @@ -223,11 +224,29 @@ def __init__( max_retries=self.settings.exemplar_retrieval_max_retries, ) - # Load guidelines (venue-aware resolution) + # Load guidelines (venue-aware resolution: built-in packs, then user packs) guidelines_path = self.settings.guidelines_path venue = self.settings.venue - self._methodology_guidelines = load_methodology_guidelines(guidelines_path, venue=venue) - self._plot_guidelines = load_plot_guidelines(guidelines_path, venue=venue) + venue_dir = self.settings.venue_dir + self._venue_pack: VenuePack | None = None + if venue and venue != "custom": + self._venue_pack = resolve_venue( + venue, builtin_dir=guidelines_path, extra_dir=venue_dir + ) + self._methodology_guidelines = load_methodology_guidelines( + guidelines_path, venue=venue, venue_dir=venue_dir + ) + self._plot_guidelines = load_plot_guidelines( + guidelines_path, venue=venue, venue_dir=venue_dir + ) + if self._venue_pack and self._venue_pack.config.fonts: + font_note = ( + "\n\n## Venue Font Preferences\n\n" + f"Preferred font families for this venue: " + f"{', '.join(self._venue_pack.config.fonts)}.\n" + ) + self._methodology_guidelines += font_note + self._plot_guidelines += font_note # Initialize agents prompt_dir = self._find_prompt_dir() @@ -1545,10 +1564,13 @@ async def generate( # ── Phase 2: Iterative Refinement ───────────────────────────── - # Aspect ratio priority: user-specified > planner-recommended > default (None) - effective_ratio = input.aspect_ratio or planner_ratio + # Aspect ratio priority: user-specified > venue default (venue.yaml) + # > planner-recommended > default (None) + venue_ratio = self._venue_pack.config.aspect_ratio if self._venue_pack else None + effective_ratio, ratio_source = select_aspect_ratio( + input.aspect_ratio, venue_ratio, planner_ratio + ) if effective_ratio: - ratio_source = "user" if input.aspect_ratio else "planner" logger.info( "Using aspect ratio", source=ratio_source, diff --git a/paperbanana/core/workflow_runner.py b/paperbanana/core/workflow_runner.py index bc3b232e..a25920ed 100644 --- a/paperbanana/core/workflow_runner.py +++ b/paperbanana/core/workflow_runner.py @@ -30,6 +30,7 @@ from paperbanana.core.source_loader import load_methodology_source from paperbanana.core.types import DiagramType, GenerationInput from paperbanana.core.utils import ensure_dir +from paperbanana.guidelines.venues import validate_venue logger = structlog.get_logger() @@ -91,8 +92,7 @@ def run_methodology_batch( manifest_path = Path(manifest_path).resolve() if format not in ("png", "jpeg", "webp"): raise ValueError(f"Format must be png, jpeg, or webp. Got: {format}") - if venue and venue.lower() not in ("neurips", "icml", "acl", "ieee", "custom"): - raise ValueError(f"venue must be neurips, icml, acl, ieee, or custom. Got: {venue}") + validate_venue(venue) if max_retries < 0: raise ValueError("max_retries must be >= 0") if concurrency < 1: @@ -335,8 +335,7 @@ def run_plot_batch( manifest_path = Path(manifest_path).resolve() if format not in ("png", "jpeg", "webp"): raise ValueError(f"Format must be png, jpeg, or webp. Got: {format}") - if venue and venue.lower() not in ("neurips", "icml", "acl", "ieee", "custom"): - raise ValueError(f"venue must be neurips, icml, acl, ieee, or custom. Got: {venue}") + validate_venue(venue) if max_retries < 0: raise ValueError("max_retries must be >= 0") if concurrency < 1: @@ -550,8 +549,7 @@ def run_orchestration_package( is_resume = bool(resume_orchestrate) if format not in ("png", "jpeg", "webp"): raise ValueError(f"Format must be png, jpeg, or webp. Got: {format}") - if venue and venue.lower() not in ("neurips", "icml", "acl", "ieee", "custom"): - raise ValueError(f"venue must be neurips, icml, acl, ieee, or custom. Got: {venue}") + validate_venue(venue) if max_method_figures < 1: raise ValueError("max_method_figures must be >= 1") if max_plot_figures < 0: diff --git a/paperbanana/guidelines/methodology.py b/paperbanana/guidelines/methodology.py index 2144e462..ebec5d33 100644 --- a/paperbanana/guidelines/methodology.py +++ b/paperbanana/guidelines/methodology.py @@ -101,30 +101,47 @@ def load_methodology_guidelines( guidelines_path: str | None = None, venue: str | None = None, + venue_dir: str | None = None, ) -> str: """Load methodology diagram style guidelines. Args: - guidelines_path: Base directory for guideline files. If None, uses defaults. - venue: Target venue (neurips, icml, acl, ieee). When set to "custom" or - None, the loader skips venue subdirectory resolution and looks for - files directly under guidelines_path (original behavior). + guidelines_path: Base directory for built-in guideline files. If None, + uses defaults. + venue: Target venue pack name (built-in such as neurips/icml/acl/ieee, + or a user pack). When set to "custom" or None, the loader skips + venue resolution and looks for files directly under guidelines_path + (original behavior). Unknown venue names raise + :class:`~paperbanana.guidelines.venues.UnknownVenueError`. + venue_dir: User venue directory override (default: PAPERBANANA_VENUE_DIR + env var, then ~/.config/paperbanana/venues). Returns: Guidelines text. """ - if guidelines_path: - base = Path(guidelines_path) - - # Try venue-specific path first: {guidelines_path}/{venue}/methodology_style_guide.md - if venue and venue != "custom": - venue_path = base / venue / "methodology_style_guide.md" - if venue_path.exists(): - logger.info("Loading methodology guidelines", venue=venue, path=str(venue_path)) - return venue_path.read_text(encoding="utf-8") + if venue and venue != "custom": + from paperbanana.guidelines.venues import resolve_venue + + pack = resolve_venue(venue, builtin_dir=guidelines_path, extra_dir=venue_dir) + if pack.methodology_guide_path: + logger.info( + "Loading methodology guidelines", + venue=pack.name, + source=pack.source, + path=str(pack.methodology_guide_path), + ) + return pack.methodology_guide_path.read_text(encoding="utf-8") + if pack.name != "neurips": + logger.warning( + "Venue pack has no methodology_style_guide.md; using built-in defaults", + venue=pack.name, + pack_dir=str(pack.dir), + ) + return DEFAULT_METHODOLOGY_GUIDELINES - # Fallback to flat path: {guidelines_path}/methodology_style_guide.md - flat_path = base / "methodology_style_guide.md" + if guidelines_path: + # Flat path: {guidelines_path}/methodology_style_guide.md + flat_path = Path(guidelines_path) / "methodology_style_guide.md" if flat_path.exists(): logger.info("Loading methodology guidelines (flat path)", path=str(flat_path)) return flat_path.read_text(encoding="utf-8") diff --git a/paperbanana/guidelines/plots.py b/paperbanana/guidelines/plots.py index 36cce52c..b4231888 100644 --- a/paperbanana/guidelines/plots.py +++ b/paperbanana/guidelines/plots.py @@ -97,30 +97,47 @@ def load_plot_guidelines( guidelines_path: str | None = None, venue: str | None = None, + venue_dir: str | None = None, ) -> str: """Load statistical plot style guidelines. Args: - guidelines_path: Base directory for guideline files. If None, uses defaults. - venue: Target venue (neurips, icml, acl, ieee). When set to "custom" or - None, the loader skips venue subdirectory resolution and looks for - files directly under guidelines_path (original behavior). + guidelines_path: Base directory for built-in guideline files. If None, + uses defaults. + venue: Target venue pack name (built-in such as neurips/icml/acl/ieee, + or a user pack). When set to "custom" or None, the loader skips + venue resolution and looks for files directly under guidelines_path + (original behavior). Unknown venue names raise + :class:`~paperbanana.guidelines.venues.UnknownVenueError`. + venue_dir: User venue directory override (default: PAPERBANANA_VENUE_DIR + env var, then ~/.config/paperbanana/venues). Returns: Guidelines text. """ - if guidelines_path: - base = Path(guidelines_path) - - # Try venue-specific path first: {guidelines_path}/{venue}/plot_style_guide.md - if venue and venue != "custom": - venue_path = base / venue / "plot_style_guide.md" - if venue_path.exists(): - logger.info("Loading plot guidelines", venue=venue, path=str(venue_path)) - return venue_path.read_text(encoding="utf-8") + if venue and venue != "custom": + from paperbanana.guidelines.venues import resolve_venue + + pack = resolve_venue(venue, builtin_dir=guidelines_path, extra_dir=venue_dir) + if pack.plot_guide_path: + logger.info( + "Loading plot guidelines", + venue=pack.name, + source=pack.source, + path=str(pack.plot_guide_path), + ) + return pack.plot_guide_path.read_text(encoding="utf-8") + if pack.name != "neurips": + logger.warning( + "Venue pack has no plot_style_guide.md; using built-in defaults", + venue=pack.name, + pack_dir=str(pack.dir), + ) + return DEFAULT_PLOT_GUIDELINES - # Fallback to flat path: {guidelines_path}/plot_style_guide.md - flat_path = base / "plot_style_guide.md" + if guidelines_path: + # Flat path: {guidelines_path}/plot_style_guide.md + flat_path = Path(guidelines_path) / "plot_style_guide.md" if flat_path.exists(): logger.info("Loading plot guidelines (flat path)", path=str(flat_path)) return flat_path.read_text(encoding="utf-8") diff --git a/paperbanana/guidelines/venues.py b/paperbanana/guidelines/venues.py new file mode 100644 index 00000000..5c9ed45c --- /dev/null +++ b/paperbanana/guidelines/venues.py @@ -0,0 +1,309 @@ +"""Venue style pack resolution. + +A *venue style pack* is a directory containing: + +- ``methodology_style_guide.md`` — style guide for methodology diagrams +- ``plot_style_guide.md`` — style guide for statistical plots +- ``venue.yaml`` (optional) — venue metadata, see :class:`VenueConfig` + +Packs are resolved from two sources, in order: + +1. **Built-in** packs shipped with PaperBanana under ``data/guidelines/``. + The ``neurips`` pack is special-cased: its guides live as flat files + directly under ``data/guidelines/`` and it is always available (falling + back to the baked-in default guidelines if the files are missing). +2. **User** packs from a user venue directory, resolved as: + an explicit ``--venue-dir`` flag / ``extra_dir`` argument, else the + ``PAPERBANANA_VENUE_DIR`` environment variable, else + ``~/.config/paperbanana/venues``. + +On a name clash, the **built-in pack wins** — user packs cannot shadow +built-in venues. The name ``custom`` is reserved (it bypasses venue +resolution entirely and loads flat files from the guidelines path). + +``venue.yaml`` schema (all fields optional):: + + display_name: "NeurIPS 2025" # human-readable name for listings + aspect_ratio: "16:9" # default --aspect-ratio for this venue + fonts: # preferred font families, appended as a + - "Helvetica" # note to the venue's style guides + - "Arial" +""" + +from __future__ import annotations + +import os +from pathlib import Path +from typing import Literal, Optional + +import structlog +import yaml +from pydantic import BaseModel, field_validator + +from paperbanana.core.types import SUPPORTED_ASPECT_RATIOS + +logger = structlog.get_logger() + +METHODOLOGY_GUIDE_FILENAME = "methodology_style_guide.md" +PLOT_GUIDE_FILENAME = "plot_style_guide.md" +VENUE_CONFIG_FILENAME = "venue.yaml" + +DEFAULT_BUILTIN_GUIDELINES_DIR = "data/guidelines" +VENUE_DIR_ENV_VAR = "PAPERBANANA_VENUE_DIR" +DEFAULT_USER_VENUE_DIR = Path.home() / ".config" / "paperbanana" / "venues" + +#: Reserved venue names that never resolve to a pack directory. +RESERVED_VENUE_NAMES = frozenset({"custom"}) + +VenueSource = Literal["built-in", "user"] + + +class UnknownVenueError(ValueError): + """Raised when a venue name does not resolve to any style pack.""" + + def __init__(self, name: str, builtin_names: list[str], user_names: list[str], user_dir: Path): + self.name = name + self.builtin_names = builtin_names + self.user_names = user_names + self.user_dir = user_dir + builtin_part = ", ".join(builtin_names) if builtin_names else "(none)" + user_part = ", ".join(user_names) if user_names else "(none)" + super().__init__( + f"Unknown venue '{name}'. " + f"Built-in venues: {builtin_part}. " + f"User venues in {user_dir}: {user_part}. " + "Run 'paperbanana venues list' to see all packs, or " + "'paperbanana venues init ' to scaffold a new one." + ) + + +class VenueConfig(BaseModel): + """Optional venue metadata parsed from ``venue.yaml``. + + All fields are optional; an absent or empty ``venue.yaml`` yields the + defaults below. + """ + + display_name: Optional[str] = None + aspect_ratio: Optional[str] = None + fonts: Optional[list[str]] = None + + model_config = {"extra": "ignore"} + + @field_validator("aspect_ratio") + @classmethod + def validate_aspect_ratio(cls, v: Optional[str]) -> Optional[str]: + """Ensure aspect_ratio, when provided, is one of the supported values.""" + if v is None: + return v + if v not in SUPPORTED_ASPECT_RATIOS: + supported = ", ".join(sorted(SUPPORTED_ASPECT_RATIOS)) + raise ValueError(f"venue.yaml aspect_ratio must be one of: {supported}. Got: {v}") + return v + + +class VenuePack(BaseModel): + """A resolved venue style pack.""" + + name: str + dir: Path + source: VenueSource + methodology_guide_path: Optional[Path] = None + plot_guide_path: Optional[Path] = None + config: VenueConfig = VenueConfig() + + +class VenueInfo(BaseModel): + """A venue listing entry (name, location, and source).""" + + name: str + dir: Path + source: VenueSource + + +def resolve_user_venue_dir(extra_dir: str | Path | None = None) -> Path: + """Resolve the user venue directory. + + Precedence: explicit ``extra_dir`` > ``PAPERBANANA_VENUE_DIR`` env var > + ``~/.config/paperbanana/venues``. + """ + if extra_dir: + return Path(extra_dir).expanduser() + env_dir = os.environ.get(VENUE_DIR_ENV_VAR) + if env_dir: + return Path(env_dir).expanduser() + return DEFAULT_USER_VENUE_DIR + + +def _is_pack_dir(path: Path) -> bool: + """A directory qualifies as a venue pack if it has at least one pack file.""" + return ( + (path / METHODOLOGY_GUIDE_FILENAME).is_file() + or (path / PLOT_GUIDE_FILENAME).is_file() + or (path / VENUE_CONFIG_FILENAME).is_file() + ) + + +def _scan_pack_dirs(root: Path) -> dict[str, Path]: + """Map venue name -> pack directory for all pack subdirectories of root.""" + packs: dict[str, Path] = {} + if not root.is_dir(): + return packs + for child in sorted(root.iterdir()): + name = child.name.lower() + if child.is_dir() and name not in RESERVED_VENUE_NAMES and _is_pack_dir(child): + packs.setdefault(name, child) + return packs + + +def _builtin_packs(builtin_dir: str | Path | None) -> dict[str, Path]: + """Built-in packs: venue subdirectories plus the implicit flat neurips pack.""" + base = Path(builtin_dir) if builtin_dir else Path(DEFAULT_BUILTIN_GUIDELINES_DIR) + packs = _scan_pack_dirs(base) + # NeurIPS is the project default and is served by the flat files directly + # under the guidelines dir. It is always available: when the files are + # missing the loaders fall back to the baked-in default guidelines. + packs.setdefault("neurips", base) + return packs + + +def list_venues( + builtin_dir: str | Path | None = None, + extra_dir: str | Path | None = None, +) -> dict[str, VenueInfo]: + """List all available venue packs from both sources. + + On a name clash, the built-in pack wins (user packs cannot shadow + built-in venues). + + Args: + builtin_dir: Built-in guidelines directory (default: ``data/guidelines``). + extra_dir: User venue directory override (default: env var, then + ``~/.config/paperbanana/venues``). + + Returns: + Mapping of venue name to :class:`VenueInfo`, sorted by name. + """ + venues: dict[str, VenueInfo] = {} + for name, path in _scan_pack_dirs(resolve_user_venue_dir(extra_dir)).items(): + venues[name] = VenueInfo(name=name, dir=path, source="user") + for name, path in _builtin_packs(builtin_dir).items(): + venues[name] = VenueInfo(name=name, dir=path, source="built-in") + return dict(sorted(venues.items())) + + +def load_venue_config(pack_dir: Path) -> VenueConfig: + """Parse ``venue.yaml`` from a pack directory. + + An absent or empty file yields default (all-None) config. A file whose + top level is not a mapping raises ``ValueError``. + """ + config_path = pack_dir / VENUE_CONFIG_FILENAME + if not config_path.is_file(): + return VenueConfig() + raw = yaml.safe_load(config_path.read_text(encoding="utf-8")) + if raw is None: + return VenueConfig() + if not isinstance(raw, dict): + raise ValueError(f"Invalid venue.yaml (expected a mapping at top level): {config_path}") + return VenueConfig(**raw) + + +def resolve_venue( + name: str, + builtin_dir: str | Path | None = None, + extra_dir: str | Path | None = None, +) -> VenuePack: + """Resolve a venue name to a style pack. + + Built-in packs are checked first, then user packs. Unknown names raise + :class:`UnknownVenueError` listing the venues available from both sources. + + Args: + name: Venue name (case-insensitive). ``custom`` is reserved and not + resolvable. + builtin_dir: Built-in guidelines directory (default: ``data/guidelines``). + extra_dir: User venue directory override (default: env var, then + ``~/.config/paperbanana/venues``). + + Returns: + The resolved :class:`VenuePack`. + """ + normalized = name.strip().lower() + if normalized in RESERVED_VENUE_NAMES: + raise ValueError( + f"Venue name '{normalized}' is reserved: 'custom' bypasses venue resolution " + "and loads flat guideline files from the guidelines path." + ) + + builtins = _builtin_packs(builtin_dir) + user_dir = resolve_user_venue_dir(extra_dir) + users = _scan_pack_dirs(user_dir) + + if normalized in builtins: + pack_dir = builtins[normalized] + source: VenueSource = "built-in" + if normalized in users: + logger.warning( + "User venue pack is shadowed by a built-in venue of the same name", + venue=normalized, + user_pack=str(users[normalized]), + ) + elif normalized in users: + pack_dir = users[normalized] + source = "user" + else: + raise UnknownVenueError( + normalized, + builtin_names=sorted(builtins), + user_names=sorted(users), + user_dir=user_dir, + ) + + methodology_path = pack_dir / METHODOLOGY_GUIDE_FILENAME + plot_path = pack_dir / PLOT_GUIDE_FILENAME + return VenuePack( + name=normalized, + dir=pack_dir, + source=source, + methodology_guide_path=methodology_path if methodology_path.is_file() else None, + plot_guide_path=plot_path if plot_path.is_file() else None, + config=load_venue_config(pack_dir), + ) + + +def validate_venue( + venue: str | None, + builtin_dir: str | Path | None = None, + extra_dir: str | Path | None = None, +) -> None: + """Validate a venue name, raising :class:`UnknownVenueError` if unknown. + + ``None`` and ``custom`` are accepted without resolution. + """ + if not venue or venue.strip().lower() in RESERVED_VENUE_NAMES: + return + resolve_venue(venue, builtin_dir=builtin_dir, extra_dir=extra_dir) + + +def select_aspect_ratio( + user_ratio: str | None, + venue_ratio: str | None, + planner_ratio: str | None, +) -> tuple[str | None, str | None]: + """Pick the effective aspect ratio and its source. + + Priority: user-specified > venue default (from ``venue.yaml``) > + planner-recommended. + + Returns: + Tuple of ``(effective_ratio, source)`` where source is one of + ``"user"``, ``"venue"``, ``"planner"`` or ``None``. + """ + if user_ratio: + return user_ratio, "user" + if venue_ratio: + return venue_ratio, "venue" + if planner_ratio: + return planner_ratio, "planner" + return None, None diff --git a/paperbanana/studio/app.py b/paperbanana/studio/app.py index 493d3e8a..c043b21e 100644 --- a/paperbanana/studio/app.py +++ b/paperbanana/studio/app.py @@ -45,6 +45,13 @@ def _upload_path(file_obj: Any) -> Optional[str]: return getattr(file_obj, "name", None) or str(file_obj) +def _venue_choices() -> list[str]: + """Available venue style packs (built-in and user) plus 'custom'.""" + from paperbanana.guidelines.venues import list_venues + + return sorted(list_venues()) + ["custom"] + + def build_studio_app( *, default_output_dir: str = "outputs", @@ -745,7 +752,7 @@ def _do_batch( ) o_venue = gr.Dropdown( label="Venue style", - choices=["neurips", "icml", "acl", "ieee", "custom"], + choices=_venue_choices(), value="neurips", ) with gr.Row(): diff --git a/paperbanana/studio/runner.py b/paperbanana/studio/runner.py index 0d53fae8..605c3a61 100644 --- a/paperbanana/studio/runner.py +++ b/paperbanana/studio/runner.py @@ -783,9 +783,9 @@ def emit(msg: str) -> None: pages_arg = None cfg = (config_path or "").strip() or None + # Venue names (built-in and user packs) are validated downstream by + # run_orchestration_package; unknown names raise listing available venues. venue_s = (venue or "neurips").strip().lower() - if venue_s not in ("neurips", "icml", "acl", "ieee", "custom"): - venue_s = "neurips" max_m = max(1, int(max_method_figures or 1)) max_p = max(0, int(max_plot_figures or 0)) diff --git a/tests/test_venue.py b/tests/test_venue.py index 54bb702c..5a442702 100644 --- a/tests/test_venue.py +++ b/tests/test_venue.py @@ -7,7 +7,6 @@ import pytest import yaml -from pydantic import ValidationError from paperbanana.core.config import Settings from paperbanana.guidelines.methodology import ( @@ -18,6 +17,7 @@ DEFAULT_PLOT_GUIDELINES, load_plot_guidelines, ) +from paperbanana.guidelines.venues import UnknownVenueError # ── Settings & Validation ──────────────────────────────────────────── @@ -38,9 +38,10 @@ def test_venue_is_case_insensitive(self): settings = Settings(venue="ICML") assert settings.venue == "icml" - def test_invalid_venue_rejected(self): - with pytest.raises(ValidationError, match="venue must be neurips"): - Settings(venue="cvpr") + def test_arbitrary_venue_names_accepted_and_normalized(self): + """Venue names are open (user packs); validation happens at resolution.""" + settings = Settings(venue=" MyLab ") + assert settings.venue == "mylab" def test_venue_from_yaml(self): with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: @@ -92,10 +93,11 @@ def test_venue_specific_path_resolved(self, guidelines_tree): result = load_methodology_guidelines(str(guidelines_tree), venue="icml") assert result == "icml-methodology" - def test_falls_back_to_flat_path(self, guidelines_tree): - """When venue subdir is missing, fall back to flat file.""" - result = load_methodology_guidelines(str(guidelines_tree), venue="unknown_venue") - assert result == "flat-methodology" + def test_unknown_venue_raises(self, guidelines_tree, monkeypatch, tmp_path): + """Unknown venues raise instead of silently falling back to flat files.""" + monkeypatch.setenv("PAPERBANANA_VENUE_DIR", str(tmp_path / "empty_user_dir")) + with pytest.raises(UnknownVenueError, match="Unknown venue 'unknown_venue'"): + load_methodology_guidelines(str(guidelines_tree), venue="unknown_venue") def test_custom_venue_uses_flat_path(self, guidelines_tree): """venue='custom' skips venue subdirectory resolution.""" @@ -106,8 +108,9 @@ def test_no_venue_uses_flat_path(self, guidelines_tree): result = load_methodology_guidelines(str(guidelines_tree), venue=None) assert result == "flat-methodology" - def test_no_path_returns_hardcoded_default(self): - result = load_methodology_guidelines(None, venue="icml") + def test_no_path_returns_hardcoded_default(self, monkeypatch, tmp_path): + monkeypatch.chdir(tmp_path) + result = load_methodology_guidelines(None, venue="neurips") assert result == DEFAULT_METHODOLOGY_GUIDELINES def test_missing_directory_returns_hardcoded_default(self): @@ -127,9 +130,10 @@ def test_venue_specific_path_resolved(self, guidelines_tree): result = load_plot_guidelines(str(guidelines_tree), venue="ieee") assert result == "ieee-plot" - def test_falls_back_to_flat_path(self, guidelines_tree): - result = load_plot_guidelines(str(guidelines_tree), venue="unknown_venue") - assert result == "flat-plot" + def test_unknown_venue_raises(self, guidelines_tree, monkeypatch, tmp_path): + monkeypatch.setenv("PAPERBANANA_VENUE_DIR", str(tmp_path / "empty_user_dir")) + with pytest.raises(UnknownVenueError, match="Unknown venue 'unknown_venue'"): + load_plot_guidelines(str(guidelines_tree), venue="unknown_venue") def test_custom_venue_uses_flat_path(self, guidelines_tree): result = load_plot_guidelines(str(guidelines_tree), venue="custom") @@ -139,8 +143,9 @@ def test_no_venue_uses_flat_path(self, guidelines_tree): result = load_plot_guidelines(str(guidelines_tree), venue=None) assert result == "flat-plot" - def test_no_path_returns_hardcoded_default(self): - result = load_plot_guidelines(None, venue="acl") + def test_no_path_returns_hardcoded_default(self, monkeypatch, tmp_path): + monkeypatch.chdir(tmp_path) + result = load_plot_guidelines(None, venue="neurips") assert result == DEFAULT_PLOT_GUIDELINES def test_missing_directory_returns_hardcoded_default(self): diff --git a/tests/test_venue_packs.py b/tests/test_venue_packs.py new file mode 100644 index 00000000..4fcb2b41 --- /dev/null +++ b/tests/test_venue_packs.py @@ -0,0 +1,399 @@ +"""Tests for user-suppliable venue style packs (resolver, venue.yaml, CLI).""" + +from __future__ import annotations + +import re + +import pytest +import yaml +from typer.testing import CliRunner + +from paperbanana.cli import app +from paperbanana.core.config import Settings +from paperbanana.core.pipeline import PaperBananaPipeline +from paperbanana.guidelines.methodology import ( + DEFAULT_METHODOLOGY_GUIDELINES, + load_methodology_guidelines, +) +from paperbanana.guidelines.plots import DEFAULT_PLOT_GUIDELINES, load_plot_guidelines +from paperbanana.guidelines.venues import ( + UnknownVenueError, + VenueConfig, + list_venues, + load_venue_config, + resolve_user_venue_dir, + resolve_venue, + select_aspect_ratio, + validate_venue, +) + +runner = CliRunner() + + +def _strip_ansi(text: str) -> str: + return re.sub(r"\x1b\[[0-9;]*m", "", text) + + +def _make_pack(root, name, methodology="m-guide", plot="p-guide", config=None): + """Create a venue pack directory under root.""" + pack = root / name + pack.mkdir(parents=True) + if methodology is not None: + (pack / "methodology_style_guide.md").write_text(methodology) + if plot is not None: + (pack / "plot_style_guide.md").write_text(plot) + if config is not None: + (pack / "venue.yaml").write_text(config) + return pack + + +@pytest.fixture() +def builtin_dir(tmp_path): + """Built-in guidelines layout: flat neurips files + venue subdirs.""" + base = tmp_path / "builtin" + base.mkdir() + (base / "methodology_style_guide.md").write_text("neurips-m") + (base / "plot_style_guide.md").write_text("neurips-p") + for venue in ("icml", "acl", "ieee"): + _make_pack(base, venue, methodology=f"{venue}-m", plot=f"{venue}-p") + return base + + +@pytest.fixture() +def user_dir(tmp_path, monkeypatch): + """Isolated user venue directory (also set as the env default).""" + d = tmp_path / "user_venues" + d.mkdir() + monkeypatch.setenv("PAPERBANANA_VENUE_DIR", str(d)) + return d + + +# ── User venue directory resolution ────────────────────────────────── + + +class TestUserVenueDirResolution: + def test_explicit_dir_wins_over_env(self, tmp_path, monkeypatch): + monkeypatch.setenv("PAPERBANANA_VENUE_DIR", str(tmp_path / "from_env")) + assert resolve_user_venue_dir(tmp_path / "explicit") == tmp_path / "explicit" + + def test_env_var_used_when_no_explicit_dir(self, tmp_path, monkeypatch): + monkeypatch.setenv("PAPERBANANA_VENUE_DIR", str(tmp_path / "from_env")) + assert resolve_user_venue_dir() == tmp_path / "from_env" + + def test_default_is_xdg_config_dir(self, monkeypatch): + monkeypatch.delenv("PAPERBANANA_VENUE_DIR", raising=False) + assert str(resolve_user_venue_dir()).endswith(".config/paperbanana/venues") + + +# ── Listing & resolution precedence ────────────────────────────────── + + +class TestListVenues: + def test_lists_builtin_and_user_with_sources(self, builtin_dir, user_dir): + _make_pack(user_dir, "mylab") + venues = list_venues(builtin_dir=builtin_dir, extra_dir=user_dir) + assert venues["neurips"].source == "built-in" + assert venues["icml"].source == "built-in" + assert venues["mylab"].source == "user" + + def test_builtin_beats_user_on_name_clash(self, builtin_dir, user_dir): + """Documented precedence: built-in packs win over user packs.""" + _make_pack(user_dir, "icml", methodology="user-icml-m") + venues = list_venues(builtin_dir=builtin_dir, extra_dir=user_dir) + assert venues["icml"].source == "built-in" + assert venues["icml"].dir == builtin_dir / "icml" + + def test_neurips_always_listed_even_without_files(self, tmp_path, user_dir): + venues = list_venues(builtin_dir=tmp_path / "missing", extra_dir=user_dir) + assert "neurips" in venues + assert venues["neurips"].source == "built-in" + + def test_non_pack_dirs_ignored(self, builtin_dir, user_dir): + (user_dir / "random_junk").mkdir() + venues = list_venues(builtin_dir=builtin_dir, extra_dir=user_dir) + assert "random_junk" not in venues + + +class TestResolveVenue: + def test_resolves_user_pack(self, builtin_dir, user_dir): + _make_pack(user_dir, "mylab", methodology="my-m", plot="my-p") + pack = resolve_venue("mylab", builtin_dir=builtin_dir, extra_dir=user_dir) + assert pack.source == "user" + assert pack.methodology_guide_path.read_text() == "my-m" + assert pack.plot_guide_path.read_text() == "my-p" + + def test_builtin_beats_user_on_name_clash(self, builtin_dir, user_dir): + _make_pack(user_dir, "acl", methodology="user-acl-m") + pack = resolve_venue("acl", builtin_dir=builtin_dir, extra_dir=user_dir) + assert pack.source == "built-in" + assert pack.methodology_guide_path.read_text() == "acl-m" + + def test_name_is_case_insensitive(self, builtin_dir, user_dir): + pack = resolve_venue(" ICML ", builtin_dir=builtin_dir, extra_dir=user_dir) + assert pack.name == "icml" + + def test_custom_is_reserved(self, builtin_dir, user_dir): + with pytest.raises(ValueError, match="reserved"): + resolve_venue("custom", builtin_dir=builtin_dir, extra_dir=user_dir) + + def test_unknown_venue_error_lists_both_sources(self, builtin_dir, user_dir): + _make_pack(user_dir, "mylab") + with pytest.raises(UnknownVenueError) as exc_info: + resolve_venue("cvpr", builtin_dir=builtin_dir, extra_dir=user_dir) + message = str(exc_info.value) + assert "Unknown venue 'cvpr'" in message + assert "Built-in venues:" in message + for builtin in ("neurips", "icml", "acl", "ieee"): + assert builtin in message + assert "User venues" in message + assert "mylab" in message + + def test_validate_venue_accepts_none_and_custom(self, builtin_dir, user_dir): + validate_venue(None, builtin_dir=builtin_dir, extra_dir=user_dir) + validate_venue("custom", builtin_dir=builtin_dir, extra_dir=user_dir) + with pytest.raises(UnknownVenueError): + validate_venue("cvpr", builtin_dir=builtin_dir, extra_dir=user_dir) + + +# ── venue.yaml parsing ─────────────────────────────────────────────── + + +class TestVenueYaml: + def test_full_config_parsed(self, user_dir): + pack_dir = _make_pack( + user_dir, + "mylab", + config=( + 'display_name: "My Lab Style"\n' + 'aspect_ratio: "16:9"\n' + "fonts:\n - Helvetica\n - Arial\n" + ), + ) + config = load_venue_config(pack_dir) + assert config.display_name == "My Lab Style" + assert config.aspect_ratio == "16:9" + assert config.fonts == ["Helvetica", "Arial"] + + def test_absent_file_yields_defaults(self, user_dir): + pack_dir = _make_pack(user_dir, "mylab") + config = load_venue_config(pack_dir) + assert config == VenueConfig() + assert config.display_name is None + assert config.aspect_ratio is None + assert config.fonts is None + + def test_empty_file_yields_defaults(self, user_dir): + pack_dir = _make_pack(user_dir, "mylab", config="") + assert load_venue_config(pack_dir) == VenueConfig() + + def test_comment_only_file_yields_defaults(self, user_dir): + pack_dir = _make_pack(user_dir, "mylab", config="# nothing set yet\n") + assert load_venue_config(pack_dir) == VenueConfig() + + def test_non_mapping_top_level_rejected(self, user_dir): + pack_dir = _make_pack(user_dir, "mylab", config="- just\n- a\n- list\n") + with pytest.raises(ValueError, match="expected a mapping"): + load_venue_config(pack_dir) + + def test_invalid_aspect_ratio_rejected(self, user_dir): + pack_dir = _make_pack(user_dir, "mylab", config='aspect_ratio: "7:5"\n') + with pytest.raises(ValueError, match="aspect_ratio must be one of"): + load_venue_config(pack_dir) + + def test_unknown_keys_ignored(self, user_dir): + pack_dir = _make_pack(user_dir, "mylab", config="display_name: X\nfuture_field: y\n") + assert load_venue_config(pack_dir).display_name == "X" + + def test_config_attached_to_resolved_pack(self, builtin_dir, user_dir): + _make_pack(user_dir, "mylab", config='aspect_ratio: "21:9"\n') + pack = resolve_venue("mylab", builtin_dir=builtin_dir, extra_dir=user_dir) + assert pack.config.aspect_ratio == "21:9" + + +# ── Guideline loaders with user packs ──────────────────────────────── + + +class TestLoadersWithUserPacks: + def test_methodology_loaded_from_user_pack(self, builtin_dir, user_dir): + _make_pack(user_dir, "mylab", methodology="my lab methodology guide") + result = load_methodology_guidelines( + str(builtin_dir), venue="mylab", venue_dir=str(user_dir) + ) + assert result == "my lab methodology guide" + + def test_plot_loaded_from_user_pack(self, builtin_dir, user_dir): + _make_pack(user_dir, "mylab", plot="my lab plot guide") + result = load_plot_guidelines(str(builtin_dir), venue="mylab", venue_dir=str(user_dir)) + assert result == "my lab plot guide" + + def test_user_pack_found_via_env_var(self, builtin_dir, user_dir): + """Without an explicit venue_dir, PAPERBANANA_VENUE_DIR is honored.""" + _make_pack(user_dir, "mylab", methodology="env-resolved guide") + result = load_methodology_guidelines(str(builtin_dir), venue="mylab") + assert result == "env-resolved guide" + + def test_pack_missing_guide_falls_back_to_defaults_with_warning(self, builtin_dir, user_dir): + _make_pack(user_dir, "mylab", methodology="only-m", plot=None) + result = load_plot_guidelines(str(builtin_dir), venue="mylab", venue_dir=str(user_dir)) + assert result == DEFAULT_PLOT_GUIDELINES + + def test_unknown_venue_raises_not_falls_back(self, builtin_dir, user_dir): + with pytest.raises(UnknownVenueError): + load_methodology_guidelines(str(builtin_dir), venue="cvpr", venue_dir=str(user_dir)) + + +# ── Aspect ratio defaulting ────────────────────────────────────────── + + +class TestAspectRatioDefaulting: + def test_user_ratio_wins(self): + assert select_aspect_ratio("1:1", "16:9", "4:3") == ("1:1", "user") + + def test_venue_default_used_when_cli_did_not_pass_one(self): + assert select_aspect_ratio(None, "16:9", "4:3") == ("16:9", "venue") + + def test_planner_used_when_no_user_or_venue_ratio(self): + assert select_aspect_ratio(None, None, "4:3") == ("4:3", "planner") + + def test_none_when_nothing_set(self): + assert select_aspect_ratio(None, None, None) == (None, None) + + def test_pipeline_picks_up_venue_pack_and_aspect_ratio(self, builtin_dir, user_dir, tmp_path): + _make_pack( + user_dir, + "mylab", + methodology="my lab methodology guide", + plot="my lab plot guide", + config='aspect_ratio: "16:9"\nfonts:\n - Helvetica\n', + ) + settings = Settings( + venue="mylab", + venue_dir=str(user_dir), + guidelines_path=str(builtin_dir), + output_dir=str(tmp_path / "outputs"), + save_prompts=False, + ) + pipeline = PaperBananaPipeline( + settings=settings, + vlm_client=object(), + image_gen_fn=lambda *a, **k: None, + ) + assert pipeline._venue_pack is not None + assert pipeline._venue_pack.source == "user" + assert pipeline._venue_pack.config.aspect_ratio == "16:9" + assert pipeline._methodology_guidelines.startswith("my lab methodology guide") + assert "Helvetica" in pipeline._methodology_guidelines + assert pipeline._plot_guidelines.startswith("my lab plot guide") + + +# ── CLI: venues list / venues init ─────────────────────────────────── + + +class TestVenuesListCommand: + def test_lists_builtin_and_user_sources(self, user_dir, monkeypatch): + _make_pack(user_dir, "mylab", config='display_name: "My Lab"\naspect_ratio: "16:9"\n') + result = runner.invoke(app, ["venues", "list", "--venue-dir", str(user_dir)]) + out = _strip_ansi(result.output) + assert result.exit_code == 0 + assert "neurips" in out + assert "built-in" in out + assert "mylab" in out + assert "user" in out + assert "My Lab" in out + assert "16:9" in out + + +class TestVenuesInitCommand: + def test_scaffolds_pack(self, user_dir): + result = runner.invoke(app, ["venues", "init", "mylab", "--venue-dir", str(user_dir)]) + out = _strip_ansi(result.output) + assert result.exit_code == 0, out + assert "Created venue pack" in out + + pack = user_dir / "mylab" + methodology = (pack / "methodology_style_guide.md").read_text() + plot = (pack / "plot_style_guide.md").read_text() + assert len(methodology) > 100 # seeded from the NeurIPS template + assert len(plot) > 100 + config_text = (pack / "venue.yaml").read_text() + assert config_text.startswith("#") # commented template + parsed = yaml.safe_load(config_text) + assert parsed["display_name"] == "MYLAB" + + # The scaffolded pack is immediately usable. + pack_resolved = resolve_venue("mylab", extra_dir=user_dir) + assert pack_resolved.source == "user" + assert load_venue_config(pack) == VenueConfig(display_name="MYLAB") + + def test_seeded_guides_match_defaults_outside_repo(self, user_dir, monkeypatch, tmp_path): + monkeypatch.chdir(tmp_path) + result = runner.invoke(app, ["venues", "init", "mylab", "--venue-dir", str(user_dir)]) + assert result.exit_code == 0 + pack = user_dir / "mylab" + assert (pack / "methodology_style_guide.md").read_text() == DEFAULT_METHODOLOGY_GUIDELINES + assert (pack / "plot_style_guide.md").read_text() == DEFAULT_PLOT_GUIDELINES + + def test_rejects_builtin_name(self, user_dir): + result = runner.invoke(app, ["venues", "init", "neurips", "--venue-dir", str(user_dir)]) + out = _strip_ansi(result.output) + assert result.exit_code == 1 + assert "already exists" in out + assert not (user_dir / "neurips").exists() + + def test_rejects_existing_user_pack(self, user_dir): + _make_pack(user_dir, "mylab") + result = runner.invoke(app, ["venues", "init", "mylab", "--venue-dir", str(user_dir)]) + out = _strip_ansi(result.output) + assert result.exit_code == 1 + assert "already exists" in out + + def test_rejects_reserved_and_invalid_names(self, user_dir): + for bad_name in ("custom", "my venue", "../escape"): + result = runner.invoke(app, ["venues", "init", bad_name, "--venue-dir", str(user_dir)]) + assert result.exit_code == 1, bad_name + + +# ── CLI: --venue validation on generate/plot ───────────────────────── + + +class TestCliVenueValidation: + def test_generate_unknown_venue_lists_available(self, user_dir): + _make_pack(user_dir, "mylab") + result = runner.invoke( + app, + ["generate", "--venue", "cvpr", "--venue-dir", str(user_dir)], + ) + out = _strip_ansi(result.output) + assert result.exit_code == 1 + assert "Unknown venue 'cvpr'" in out + assert "neurips" in out + assert "mylab" in out + + def test_plot_unknown_venue_lists_available(self, user_dir): + result = runner.invoke( + app, + [ + "plot", + "--data", + "missing.csv", + "--intent", + "x", + "--venue", + "cvpr", + "--venue-dir", + str(user_dir), + ], + ) + out = _strip_ansi(result.output) + assert result.exit_code == 1 + assert "Unknown venue 'cvpr'" in out + + def test_generate_accepts_user_venue_name(self, user_dir): + """A user pack passes --venue validation (failure later is unrelated).""" + _make_pack(user_dir, "mylab") + result = runner.invoke( + app, + ["generate", "--venue", "mylab", "--venue-dir", str(user_dir), "--dry-run"], + ) + out = _strip_ansi(result.output) + assert "Unknown venue" not in out From eb8d04224f8e9ba96d7cf432bc01e8d750dc01ea Mon Sep 17 00:00:00 2001 From: dippatel1994 Date: Thu, 11 Jun 2026 19:44:32 -0400 Subject: [PATCH 2/2] fix: Windows-safe path assertion; ship venue.yaml for built-in venues - test_default_is_xdg_config_dir compared a /-joined string, failing on Windows separators; compare Path.parts instead - icml/ieee/acl gain venue.yaml: two-column venues now default figures to 4:3 (column-friendly) with venue-typical font preferences, so the config layer carries real venue knowledge, not just the guides --- data/guidelines/acl/venue.yaml | 3 +++ data/guidelines/icml/venue.yaml | 4 ++++ data/guidelines/ieee/venue.yaml | 4 ++++ tests/test_venue_packs.py | 17 ++++++++++++++++- 4 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 data/guidelines/acl/venue.yaml create mode 100644 data/guidelines/icml/venue.yaml create mode 100644 data/guidelines/ieee/venue.yaml diff --git a/data/guidelines/acl/venue.yaml b/data/guidelines/acl/venue.yaml new file mode 100644 index 00000000..0281c8df --- /dev/null +++ b/data/guidelines/acl/venue.yaml @@ -0,0 +1,3 @@ +display_name: "ACL" +# ACL *ACL venues are two-column; compact figures read better. +aspect_ratio: "4:3" diff --git a/data/guidelines/icml/venue.yaml b/data/guidelines/icml/venue.yaml new file mode 100644 index 00000000..a8064b06 --- /dev/null +++ b/data/guidelines/icml/venue.yaml @@ -0,0 +1,4 @@ +display_name: "ICML" +# ICML is two-column; column-width figures favor compact ratios. +aspect_ratio: "4:3" +fonts: ["Helvetica", "Arial"] diff --git a/data/guidelines/ieee/venue.yaml b/data/guidelines/ieee/venue.yaml new file mode 100644 index 00000000..e8936725 --- /dev/null +++ b/data/guidelines/ieee/venue.yaml @@ -0,0 +1,4 @@ +display_name: "IEEE" +# Most IEEE transactions are two-column; design for column width. +aspect_ratio: "4:3" +fonts: ["Arial", "Helvetica"] diff --git a/tests/test_venue_packs.py b/tests/test_venue_packs.py index 4fcb2b41..39459e1c 100644 --- a/tests/test_venue_packs.py +++ b/tests/test_venue_packs.py @@ -82,7 +82,8 @@ def test_env_var_used_when_no_explicit_dir(self, tmp_path, monkeypatch): def test_default_is_xdg_config_dir(self, monkeypatch): monkeypatch.delenv("PAPERBANANA_VENUE_DIR", raising=False) - assert str(resolve_user_venue_dir()).endswith(".config/paperbanana/venues") + # Compare path components, not a string — separators differ on Windows. + assert resolve_user_venue_dir().parts[-3:] == (".config", "paperbanana", "venues") # ── Listing & resolution precedence ────────────────────────────────── @@ -397,3 +398,17 @@ def test_generate_accepts_user_venue_name(self, user_dir): ) out = _strip_ansi(result.output) assert "Unknown venue" not in out + + +class TestBuiltinVenueConfigs: + """Built-in venues ship venue.yaml with column-aware defaults.""" + + def test_two_column_venues_default_to_4_3(self): + for name in ("icml", "ieee", "acl"): + pack = resolve_venue(name) + assert pack.source == "built-in" + assert pack.config.aspect_ratio == "4:3", f"{name} should default to 4:3" + + def test_font_preferences_loaded(self): + assert resolve_venue("icml").config.fonts == ["Helvetica", "Arial"] + assert resolve_venue("ieee").config.fonts == ["Arial", "Helvetica"]