Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #618 +/- ##
=======================================
Coverage 85.34% 85.34%
=======================================
Files 38 38
Lines 3397 3397
Branches 3397 3397
=======================================
Hits 2899 2899
Misses 305 305
Partials 193 193 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the input format documentation script to use Jinja templates and adds Ruff pre-commit hooks.
- Switches the manual markdown assembly to a Jinja2-based template for better readability and maintainability
- Introduces dataclasses (
Section,File,Notes) and splits file processing intoload_sections/load_file - Adds
ruff(with auto-fix) and a Jinja extension to pre-commit and VS Code configuration
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/input_format.md.jinja | New Jinja template to generate the input format markdown |
| docs/generate_input_format_doc.py | Refactored script: replaced manual string building with Jinja, added dataclasses and helper functions |
| doc-requirements.txt | Added pyyaml and jinja2 to dependencies |
| .vscode/extensions.json | Added Jinja syntax highlighting extension |
| .pre-commit-config.yaml | Configured ruff and ruff-format hooks |
Comments suppressed due to low confidence (5)
docs/generate_input_format_doc.py:44
- Consider adding unit tests for
generate_markdown(and related data-loading functions) to verify that the Jinja template renders correctly and all sections/files load as expected.
def generate_markdown() -> str:
docs/generate_input_format_doc.py:51
- Add a docstring for
load_sectionsdescribing its responsibility, the structure of the returnedSectionobjects, and why it usesIterable.
def load_sections() -> Iterable[Section]:
docs/generate_input_format_doc.py:59
- Provide a docstring for
load_filethat explains how it transforms a schema YAML into aFiledataclass, and documents the handling of missing fields.
def load_file(path: Path) -> File:
docs/generate_input_format_doc.py:85
- Add a docstring for
fields2tableto clarify how it separates table data from notes and what the return tuple elements represent.
def fields2table(fields: list[dict[str, str]]) -> tuple[str, str | None]:
docs/generate_input_format_doc.py:31
- [nitpick] The
Filedataclass uses a field nameddescriptionto hold the CSV title; consider renaming this attribute totitlefor consistency and clearer intent.
class File:
| pyyaml | ||
| table2md | ||
| jinja2 |
There was a problem hiding this comment.
[nitpick] Alphabetize and pin versions of dependencies in doc-requirements.txt (e.g., jinja2>=3.0,<4.0) to improve readability and ensure reproducible builds.
| pyyaml | |
| table2md | |
| jinja2 | |
| jinja2>=3.0,<4.0 | |
| pyyaml>=5.4,<7.0 | |
| table2md>=1.0,<2.0 |
There was a problem hiding this comment.
Dunno if we really need to pin versions at this stage (though there are presumably versions of these packages that won't work). Maybe we should hold off until we have more Python stuff in the repo and then we can use a proper tool for dependency management, like poetry or uv.
Description
I've changed the input format doc script to use jinja templates, instead of manually constructing the markdown text in the script, which should improve maintainability and make it easier to adapt the script to also handle output files (#529). Hopefully it's a bit more readable now.
I've added the
ruffpre-commit hooks as I figure we might as well. There will be more Python stuff added in future anyway.Closes #610.
Type of change
Key checklist
$ cargo test$ cargo docFurther checks