Fix TSV formatting so GitHub's file preview works (#233) - #325
Merged
Merged
Conversation
…-Initiative#233) GitHub's TSV file preview breaks when a row has a different number of columns than the header. Mixed line endings were also present in several files. Root cause: the top-level `files: src/python` restriction in .pre-commit-config.yaml scoped every hook in the file - including trailing-whitespace, mixed-line-ending, and end-of-file-fixer - to src/python/ only, so the actual PEtab data files under Benchmark-Models/ were never checked or normalized. - Add bmp-check-tsv-format (benchmark_models_petab/check_tsv_format.py), which checks every *.tsv file for: consistent field count vs. the header, consistent line endings, no stray leading/trailing whitespace in a field, and exactly one trailing newline. Wire it into tests.yml alongside bmp-petablint. - Fix the 19 files it flagged: normalize CRLF/mixed line endings to LF and add missing final newlines (via the broadened pre-commit hooks below), pad short rows in Bachmann_MSB2011/parameters_Bachmann_MSB2011.tsv (114 rows were missing their two optional prior columns entirely), and trim stray whitespace from a handful of field values. - Extend .pre-commit-config.yaml's end-of-file-fixer and mixed-line-ending hooks to also cover *.tsv anywhere in the repo, and restore the previous src/python-only scope explicitly on every other hook (check-yaml, check-added-large-files, check-merge-conflict, check-symlinks, check-executables-have-shebangs, detect-private-key, ruff, ruff-format). trailing-whitespace is deliberately NOT extended to *.tsv, since it strips trailing tabs that PEtab TSVs use to encode a legitimately empty last column - doing so would recreate the very column-mismatch bug this fixes. - Add unit tests for check_tsv_file in src/python/test/. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dweindl
marked this pull request as ready for review
September 25, 2026 09:00
dilpath
approved these changes
Sep 25, 2026
| hooks: | ||
| - id: check-yaml | ||
| description: Check yaml files for parseable syntax | ||
| files: ^src/python/ |
Collaborator
There was a problem hiding this comment.
Unfortunate that it doesn't look easy to have separate pre-commit configs for the Python code and the PEtab problems.
Member
Author
There was a problem hiding this comment.
I didn't find a more convenient option either, but I think it's quite manageable...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GitHub's TSV preview breaks when a row has a different number of columns than the header, and several files had mixed line endings. Closes #233.
Root cause: the top-level
files: src/pythonrestriction in.pre-commit-config.yamlscoped every hook in the file — includingtrailing-whitespace,mixed-line-ending, andend-of-file-fixer— tosrc/python/only, so the actual PEtab data files underBenchmark-Models/were never checked or normalized.Changes:
bmp-check-tsv-format(benchmark_models_petab/check_tsv_format.py), which checks every*.tsvfile for consistent field count vs. the header, consistent line endings, no stray leading/trailing whitespace in a field, and exactly one trailing newline. Wired intotests.ymlalongsidebmp-petablint.Bachmann_MSB2011/parameters_Bachmann_MSB2011.tsv(they were missing their two optional prior columns entirely), and trimmed stray whitespace from a handful of field values..pre-commit-config.yaml'send-of-file-fixerandmixed-line-endinghooks to also cover*.tsvanywhere in the repo, and restored the previoussrc/python-only scope explicitly on every other hook (check-yaml,check-added-large-files,check-merge-conflict,check-symlinks,check-executables-have-shebangs,detect-private-key,ruff,ruff-format).trailing-whitespaceis deliberately not extended to*.tsv, since it strips trailing tabs that PEtab TSVs use to encode a legitimately empty last column — doing so would recreate the very column-mismatch bug this fixes.check_tsv_fileinsrc/python/test/.🤖 Generated with Claude Code