Skip to content

fix: validate the topology before opening the output file - #181

Open
galjos wants to merge 1 commit into
devfrom
fix/topology-writer-truncation
Open

fix: validate the topology before opening the output file#181
galjos wants to merge 1 commit into
devfrom
fix/topology-writer-truncation

Conversation

@galjos

@galjos galjos commented Aug 23, 2026

Copy link
Copy Markdown
Member

Writing a topology that fails validation destroyed the existing file.

$ ls -l topo.top          # 34 bytes
$ python -c "write_topology_file(BondedTopology(bonds=[Bond(index1=1, index2=2)]), 'topo.top', mode='o')"
TopologyFileError: ... all bonds must have a bond type defined.
$ ls -l topo.top          # 0 bytes

The writer opened the file, which truncates in overwrite mode, and only then validated, so the error left an empty file behind. The handle was also not closed on that path. Validation now runs before the file is opened, and the write loop closes the file in a finally block.

Present in v1.5.0. 949 tests pass in both type checking modes.

TopologyFileWriter.write opened the output file before any of the bonded
parameters were checked, and the per-section checks for a missing bond,
angle, dihedral or improper type only ran from inside the write helpers.
Opening the file in overwrite mode truncates it, so a topology with a
missing type raised TopologyFileError only after the destination had
already been emptied, silently destroying an existing topology file.

The type checks now run against the sections that are about to be written
before the file is opened, so a rejected topology leaves the destination
untouched. Writing itself is wrapped in a try/finally so that the file
handle is closed even when a later write fails.
@galjos
galjos requested a review from 97gamjak August 23, 2026 19:36
@github-actions

Copy link
Copy Markdown
Contributor

PYLINT REPORT

Your code has been rated at 9.78/10

Full report

Raw metrics

type number % previous difference
code 15664 45.60 NC NC
docstring 13072 38.05 NC NC
comment 463 1.35 NC NC
empty 5155 15.01 NC NC

Duplication

now previous difference
nb duplicated lines 0 NC NC
percent duplicated lines 0.000 NC NC

Messages by category

type number previous difference
convention 15 NC NC
refactor 135 NC NC
warning 17 NC NC
error 5 NC NC

% errors / warnings by module

module error warning refactor convention
PQAnalysis.type_checking 40.00 0.00 0.00 0.00
PQAnalysis.atomic_system.atomic_system 20.00 11.76 6.67 0.00
PQAnalysis.analysis.vibrational.vibrational_analysis 20.00 0.00 7.41 0.00
PQAnalysis 20.00 0.00 0.00 0.00
PQAnalysis.analysis.vacf._vacf_kernel_py 0.00 11.76 1.48 0.00
PQAnalysis.tools.traj_to_com_traj 0.00 11.76 0.00 0.00
PQAnalysis.io.traj_file._process_lines_py 0.00 11.76 0.00 0.00
PQAnalysis.io.moldescriptor_reader 0.00 11.76 0.00 0.00
PQAnalysis.tools.add_molecule 0.00 5.88 4.44 0.00
PQAnalysis.analysis.rdf._rdf_kernel_py 0.00 5.88 2.22 0.00
PQAnalysis.analysis.msd._msd_kernel_py 0.00 5.88 2.22 0.00
PQAnalysis.io.conversion_api 0.00 5.88 1.48 0.00
PQAnalysis.version 0.00 5.88 0.00 0.00
PQAnalysis.utils.custom_logging 0.00 5.88 0.00 0.00
PQAnalysis.io.write_api 0.00 5.88 0.00 0.00
PQAnalysis.io.restart_file.restart_writer 0.00 0.00 14.81 0.00
PQAnalysis.analysis.rdf.rdf 0.00 0.00 6.67 6.67
PQAnalysis.analysis.msd.msd 0.00 0.00 5.93 33.33
PQAnalysis.io.nep.nep_writer 0.00 0.00 5.19 6.67
PQAnalysis.analysis.vacf.vacf 0.00 0.00 4.44 13.33
PQAnalysis.io.traj_file._slab_parser_py 0.00 0.00 3.70 0.00
PQAnalysis.analysis.vacf.spectrum 0.00 0.00 3.70 0.00
PQAnalysis.io.traj_file.trajectory_reader 0.00 0.00 2.96 0.00
PQAnalysis.topology.bonded_topology.dihedral 0.00 0.00 2.22 0.00
PQAnalysis.core.residue 0.00 0.00 2.22 0.00
PQAnalysis.topology.bonded_topology.bonded_topology 0.00 0.00 1.48 0.00
PQAnalysis.topology.bonded_topology.bond 0.00 0.00 1.48 0.00
PQAnalysis.topology.bonded_topology.angle 0.00 0.00 1.48 0.00
PQAnalysis.io.traj_file.raw_frame_reader 0.00 0.00 1.48 0.00
PQAnalysis.core.cell.cell 0.00 0.00 1.48 0.00
PQAnalysis.atomic_system._standard_properties 0.00 0.00 1.48 0.00
PQAnalysis.analysis.vacf.api 0.00 0.00 1.48 0.00
PQAnalysis.analysis.spectrum_broadening.api 0.00 0.00 1.48 0.00
PQAnalysis.analysis.momentum.api 0.00 0.00 1.48 0.00
PQAnalysis.analysis.vibrational.vibrational_input_file_reader 0.00 0.00 0.74 6.67
PQAnalysis.traj.formats 0.00 0.00 0.74 0.00
PQAnalysis.topology.topology 0.00 0.00 0.74 0.00
PQAnalysis.topology.selection 0.00 0.00 0.74 0.00
PQAnalysis.io.traj_file.frame_reader 0.00 0.00 0.74 0.00
PQAnalysis.io.restart_file.restart_reader 0.00 0.00 0.74 0.00
PQAnalysis.io.input_file_reader.pq_analysis._parse 0.00 0.00 0.74 0.00
PQAnalysis.io.input_file_reader.pq.pq_input_file_reader 0.00 0.00 0.74 0.00
PQAnalysis.io.input_file_reader.input_file_parser 0.00 0.00 0.74 0.00
PQAnalysis.io.info_file_reader 0.00 0.00 0.74 0.00
PQAnalysis.io.formats 0.00 0.00 0.74 0.00
PQAnalysis.analysis.momentum.momentum 0.00 0.00 0.74 0.00
PQAnalysis.core.atom.element 0.00 0.00 0.00 13.33
PQAnalysis.analysis.vacf.vacf_input_file_reader 0.00 0.00 0.00 6.67
PQAnalysis.analysis.rdf.rdf_input_file_reader 0.00 0.00 0.00 6.67
PQAnalysis.analysis.msd.msd_input_file_reader 0.00 0.00 0.00 6.67

Messages

message id occurrences
too-many-positional-arguments 29
too-many-arguments 29
duplicate-code 20
too-many-locals 15
too-many-instance-attributes 13
invalid-name 12
too-complex 9
inconsistent-return-statements 8
fixme 7
too-many-branches 5
missing-type-doc 4
too-many-return-statements 3
too-many-lines 3
unused-import 2
too-many-statements 2
redefined-builtin 2
missing-kwoa 2
use-set-for-membership 1
unexpected-keyword-arg 1
too-many-public-methods 1
possibly-used-before-assignment 1
no-member 1
missing-param-doc 1
arguments-differ 1

@codecov

codecov Bot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.94%. Comparing base (bd55571) to head (a8583b9).
⚠️ Report is 8 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #181      +/-   ##
==========================================
+ Coverage   93.86%   93.94%   +0.07%     
==========================================
  Files         177      177              
  Lines        9215     9231      +16     
==========================================
+ Hits         8650     8672      +22     
+ Misses        565      559       -6     
Flag Coverage Δ
unittests 93.94% <100.00%> (+0.07%) ⬆️
Files with missing lines Coverage Δ
...QAnalysis/io/topology_file/topology_file_writer.py 90.68% <100.00%> (+0.23%) ⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

"bonds": "bond",
"angles": "angle",
"dihedrals": "dihedral",
"impropers": "improper",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about j coulping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants