Skip to content

fix: accept path-like objects in tofile and validate weight format before writing#381

Open
henryiii wants to merge 1 commit into
mainfrom
fix-writer-path-and-validation
Open

fix: accept path-like objects in tofile and validate weight format before writing#381
henryiii wants to merge 1 commit into
mainfrom
fix-writer-path-and-validation

Conversation

@henryiii

Copy link
Copy Markdown
Member

🤖 AI text below 🤖

Summary

  • Bug 1 (tofile + pathlib.Path): _open_write_file called filepath.endswith(...) which raises AttributeError for pathlib.Path objects. Fixed by converting via os.fsdecode(os.fspath(filepath)) before the suffix check, and widened the type annotations on _open_write_file and LesHouchesEvents.tofile from str to the existing PathLike alias (matching the reading side).

  • Bug 2 (partial-file on validation failure): LesHouchesEvents.write() performed no up-front validation, so calling tofile(path, weights=True) with the default rwgt=True raised ValueError only when the first event was serialized — after the root tag, header, and init block had already been written, leaving a partial/corrupt file on disk. Fixed by adding the rwgt and weights mutual-exclusion check at the top of both write() and tofile(), before any I/O.

Part of #378

Test plan

  • test_tofile_accepts_pathlib_path — writes via pathlib.Path, reads back, verifies 1 event
  • test_tofile_accepts_pathlib_path_gz — writes .lhe.gz via pathlib.Path, checks gzip magic, reads back via pylhe
  • test_write_raises_before_writing_when_both_formats_given — asserts target file does NOT exist after the failed tofile() call
  • test_write_stream_raises_before_writing_when_both_formats_given — asserts StringIO is empty after the failed write() call

🤖 Generated with Claude Code

…fore writing

Bug 1: _open_write_file called filepath.endswith(...) which raises
AttributeError for pathlib.Path objects. Fixed by converting to str via
os.fsdecode(os.fspath(filepath)) before the suffix check. Widened the
type annotation on _open_write_file and LesHouchesEvents.tofile from
str to the existing PathLike alias to match the reading side (fromfile).

Bug 2: LesHouchesEvents.write() validated nothing up-front, so calling
tofile(path, weights=True) with the default rwgt=True would raise only
when the first event was serialized — after the root tag, header, and
init block had already been written, leaving a partial file on disk.
Fixed by adding the same rwgt+weights mutual-exclusion check at the
top of both write() and tofile(), before any I/O is performed.

Assisted-by: ClaudeCode:claude-sonnet-4-6
@github-actions

Copy link
Copy Markdown

🏎️ Benchmark Comparison


--------------------------------------------------------------------------------------------------- benchmark: 4 tests --------------------------------------------------------------------------------------------------
Name (time in ms)                                   Min                   Max                  Mean             StdDev                Median                IQR            Outliers     OPS            Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_count_events_benchmark (0001_base)        318.3076 (1.0)        329.7467 (1.02)       323.8630 (1.01)      3.6017 (2.24)       323.1768 (1.01)      5.8256 (2.65)          3;0  3.0877 (0.99)         10           1
test_count_events_benchmark (0002_mr)          319.0777 (1.00)       323.8940 (1.0)        321.2229 (1.0)       1.6087 (1.0)        321.5339 (1.0)       2.1992 (1.0)           5;0  3.1131 (1.0)          10           1
test_fromfile_and_to_awkward (0002_mr)       2,428.9791 (7.63)     2,494.0276 (7.70)     2,451.5306 (7.63)     20.9414 (13.02)    2,450.9120 (7.62)     30.6736 (13.95)         3;0  0.4079 (0.13)         10           1
test_fromfile_and_to_awkward (0001_base)     2,476.6205 (7.78)     2,543.4273 (7.85)     2,523.4269 (7.86)     19.2310 (11.95)    2,525.7700 (7.86)     17.1427 (7.80)          2;1  0.3963 (0.13)         10           1
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Legend:
  Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
  OPS: Operations Per Second, computed as 1 / Mean

@henryiii henryiii marked this pull request as ready for review June 12, 2026 17:57
@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (7e7441b) to head (b75d95b).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #381   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          608       615    +7     
=========================================
+ Hits           608       615    +7     
Flag Coverage Δ
unittests-3.10 100.00% <100.00%> (ø)
unittests-3.11 100.00% <100.00%> (ø)
unittests-3.12 100.00% <100.00%> (ø)
unittests-3.13 100.00% <100.00%> (ø)
unittests-3.14 100.00% <100.00%> (ø)
unittests-3.9 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

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.

1 participant