Skip to content

feat(quip/gap/xyz): implement to_labeled_system and to_multi_systems methods with file handler support#888

Merged
wanghan-iapcm merged 10 commits intodevelfrom
copilot/fix-887
Sep 3, 2025
Merged

feat(quip/gap/xyz): implement to_labeled_system and to_multi_systems methods with file handler support#888
wanghan-iapcm merged 10 commits intodevelfrom
copilot/fix-887

Conversation

Copy link
Contributor

Copilot AI commented Sep 1, 2025

Implements export functionality for QUIP/GAP XYZ format with support for both file paths and file handlers.

Key Features

  • Single System Export: to_labeled_system writes individual systems to QUIP/GAP XYZ format
  • Multi-System Export: to_multi_systems yields file handlers for writing multiple systems to a single file
  • File Handler Support: Both methods accept file paths or file handlers as input
  • Smart Write Logic: Always overwrites for file paths, appends only when using file handlers

Implementation Details

The to_multi_systems method uses a generator pattern that yields file handlers instead of filenames, enabling more flexible file management. The to_labeled_system method handles both file paths and file handlers appropriately, with different write modes for each.

When writing multiple systems to the same file handler, configurations are written directly adjacent to each other without separator lines, matching the standard QUIP/GAP XYZ format specification.

Testing

Comprehensive test suite covers:

  • Single system export functionality
  • Multi-system export to shared file
  • Roundtrip consistency verification
  • All existing QUIP/GAP XYZ tests continue to pass (121 tests)

This maintains full backward compatibility while adding the requested export capabilities.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits September 1, 2025 11:57
…methods

Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
Copilot AI changed the title [WIP] [Feature Request] implement to_labeled_system and to_multi_systems for QuipGapXYZFormat feat(quip/gap/xyz): implement to_labeled_system and to_multi_systems methods Sep 1, 2025
Copilot AI requested a review from njzjz September 1, 2025 12:00
…r reviewer feedback

Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
Copilot AI requested a review from njzjz September 1, 2025 12:25
…ove import organization

Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
Copilot AI changed the title feat(quip/gap/xyz): implement to_labeled_system and to_multi_systems methods feat(quip/gap/xyz): implement to_labeled_system and to_multi_systems methods with file handler support Sep 1, 2025
Copilot AI requested a review from njzjz September 1, 2025 12:49
njzjz and others added 2 commits September 1, 2025 20:52
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 1, 2025

CodSpeed WallTime Performance Report

Merging #888 will not alter performance

Comparing copilot/fix-887 (6dac458) with devel (beee99a)1

Summary

✅ 2 untouched benchmarks

Footnotes

  1. No successful run was found on devel (ba31bcd) during the generation of this report, so beee99a was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@njzjz
Copy link
Member

njzjz commented Sep 1, 2025

@copilot Fix unit tests. Fix pre-commit errors.

…sitioning correctly

Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 1, 2025

@copilot Fix unit tests. Fix pre-commit errors.

Fixed unit tests and addressed the file handler positioning issue. The problem was that when multiple systems were written to the same file handler via to_multi_systems, subsequent systems would overwrite each other. Added proper newline separation logic and file position checking. All 121 XYZ-related tests now pass. Commit: 8871e2f

Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
@codecov
Copy link

codecov bot commented Sep 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.94%. Comparing base (ba31bcd) to head (6dac458).
⚠️ Report is 25 commits behind head on devel.

Additional details and impacted files
@@            Coverage Diff             @@
##            devel     #888      +/-   ##
==========================================
+ Coverage   85.84%   85.94%   +0.10%     
==========================================
  Files          82       82              
  Lines        7706     7754      +48     
==========================================
+ Hits         6615     6664      +49     
+ Misses       1091     1090       -1     

☔ View full report in Codecov by Sentry.
📢 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.

@njzjz njzjz marked this pull request as ready for review September 1, 2025 14:01
…ystem export

Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
Copilot AI requested a review from njzjz September 1, 2025 14:17
@njzjz njzjz requested a review from wanghan-iapcm September 1, 2025 14:23
@wanghan-iapcm wanghan-iapcm merged commit a82693b into devel Sep 3, 2025
21 checks passed
@wanghan-iapcm wanghan-iapcm deleted the copilot/fix-887 branch September 3, 2025 00:32
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.

[Feature Request] implement to_labeled_system and to_multi_systems for QuipGapXYZFormat

3 participants