Refactor/cli template dir #111
Conversation
…of language-specific templates Missed test - template reference
…sets, and compilation. Catch remaining issues with custom directories.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| ---------- | ||
| artifact_dir : Path | ||
| Directory containing Typst artifacts (.typ files). | ||
| Directory containing Typst template files |
There was a problem hiding this comment.
Was previous documentation was better? Here .typs are treated as client specific artifacts. These are by definition typst template files, but we use the term template in the repository in other ways.
There was a problem hiding this comment.
I like use of word template better. Maybe a hybrid?
"Directory containing Typst template files (.typ)."
|
@jangevaare are we still using the --test-mode flag from my original work? Or now no need? |
|
|
||
| # Typst compilation always uses PROJECT_ROOT to find both templates and output artifacts. | ||
| # template_dir parameter is for dynamic template loading in generate_notices, not for Typst --root. | ||
| root_dir = ROOT_DIR |
There was a problem hiding this comment.
Why is this necessary if a constant?
| Path to parameters.yaml configuration file. | ||
| template_dir : Path, optional | ||
| Template directory containing conf.typ and assets/. Used as Typst --root. | ||
| If not provided, defaults to project root. |
There was a problem hiding this comment.
| If not provided, defaults to project root. | |
| If not provided, defaults to project root, runs in test mode | |
| spec.loader.exec_module(module) | ||
|
|
||
| # Validate render_notice() exists | ||
| if not hasattr(module, "render_notice"): |
There was a problem hiding this comment.
Should this be checked upon loading (load_template_module)?
| renderers = {} | ||
| for lang in Language: | ||
| module_path = template_dir / f"{lang.value}_template.py" | ||
| # Only load if template file exists |
There was a problem hiding this comment.
Wondering if this should be checked on load if it is a necessary component?
| f"Available languages: {available}\n" | ||
| f"Ensure your template directory contains {language.value}_template.py" | ||
| ) | ||
| return renderers[language.value] |
There was a problem hiding this comment.
Wondering why this is separate from build? May suggest considering combining the two for simplicity.
| def run_step_4_generate_notices( | ||
| output_dir: Path, | ||
| run_id: str, | ||
| assets_dir: Path, |
There was a problem hiding this comment.
assets_dir would still exist?
The way I have it set up is phu_templates/phu_acronym/assets ?
There was a problem hiding this comment.
I see now this is addressed down more.
| help=f"Config directory (default: {DEFAULT_CONFIG_DIR})", | ||
| ) | ||
| parser.add_argument( | ||
| "--template", |
There was a problem hiding this comment.
I'm wondering whether we call this --template_acronym or --phu_acronym for clarity.
There was a problem hiding this comment.
What about --phu_template, which matches the parent folder name (minus the s)
| * **Orchestrator:** `pipeline/orchestrator.py` is the `viper` CLI entry point and coordinates 9 steps. | ||
| * **Steps:** Modules are organized by steps 1–9, not by functional themes. | ||
| * **Templates:** `templates/` contains `en_template.py`, `fr_template.py`. Import via `from templates import ...`. Typesetting is separate from orchestration. | ||
| * **Templates:** `templates/` contains default language templates (`en_template.py`, `fr_template.py`), Typst config (`conf.typ`), and assets. `phu_templates/` is the container for PHU-specific template customizations (gitignored). Templates are loaded dynamically at runtime via `build_language_renderers()` in `generate_notices.py`. The `--template` CLI argument expects a template name within `phu_templates/` (e.g., `--template wdgph` → `phu_templates/wdgph/`). Template names cannot contain path separators (no nesting). Each template module must define a `render_notice()` function. Template directory isolation: templates, conf.typ, and assets stay together. Typesetting is separate from orchestration. |
There was a problem hiding this comment.
Really thinking that we may need to clarify template name to the phu acronym!
Co-authored-by: Kassy Raymond <44474665+kassyray@users.noreply.github.com>
No need now |
|
Wondering if we use term "demo" instead of "test" when referring to example templates we provide? |
That sounds like a good idea. |
Alternative to #93
This pull request introduces a major overhaul to how template customization and selection works in the pipeline, enabling dynamic loading of PHU-specific templates and assets at runtime. The changes allow organizations to maintain their own template directories (with custom branding, layouts, and assets) without modifying core code, and improve documentation and error handling for template management. The CLI is updated to support a
--templateargument, and the codebase now supports dynamic template discovery and loading.Template system and CLI improvements:
phu_templates/) with customizable language modules, Typst config, and assets. Templates are loaded dynamically at runtime viabuild_language_renderers()ingenerate_notices.py. The CLI now accepts a--templateargument to select a template directory. Documentation inAGENTS.MD,README.md, and the newphu_templates/README.mdexplains usage, requirements, and error handling. [1] [2] [3] [4]Dynamic template loading and error handling:
Refactored template loading in
pipeline/generate_notices.pyto useload_template_module()andbuild_language_renderers()for dynamic discovery of available templates. Improved error messages for missing templates or assets, and made it possible for PHUs to provide templates for only a subset of supported languages.Updated the rendering pipeline to pass the dynamic renderer dictionary to notice rendering functions, ensuring the correct template is used per organization and language. Improved error handling for missing language templates and assets.
Typst compilation and asset resolution:
pipeline/compile_notices.pyto distinguish between the template directory (for dynamic template loading) and the Typst root directory (for compilation). Improved asset path resolution to support both relative and absolute paths, enabling use of custom assets outside the project root. [1] [2] [3] [4] [5] [6] [7]Documentation and onboarding:
README.md,AGENTS.MD,phu_templates/README.md) to explain the new template system, CLI usage, template directory structure, asset requirements, and error handling for missing templates/assets. [1] [2]Codebase cleanup and extensibility:
These changes make the pipeline significantly more flexible and user-friendly for organizations needing custom branding and layouts, while maintaining robust error handling and clear documentation.