Adding Appending Functionality method to WriterBase #3863
Conversation
Codecov ReportBase: 94.31% // Head: 94.34% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #3863 +/- ##
===========================================
+ Coverage 94.31% 94.34% +0.03%
===========================================
Files 193 193
Lines 25024 25068 +44
Branches 3374 3376 +2
===========================================
+ Hits 23601 23651 +50
+ Misses 1374 1368 -6
Partials 49 49
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| append: bool (optional) | ||
| If ``True``, append to an existing file. If ``False``, overwrite | ||
| the file. Default is ``False``. |
There was a problem hiding this comment.
Document that this option only exists for API compatibility and will raise an issue
| ) | ||
| self.filename = filename | ||
| self.n_atoms = n_atoms | ||
| super().__init__(filename, n_atoms, append) | ||
| if mode != "a" and mode != "w": | ||
| raise IOError("Expected 'a' or 'w' as mode in ChemfilesWriter") |
There was a problem hiding this comment.
ChemFiles is possibly using 'a' for append mode here
There was a problem hiding this comment.
It's up for discussion but my idea is to focus this PR on only adding general API to WriterBase and leave specific implementations to other PRs (in a batch for similar Writers) so it doesn't get messy :)
Also after this PR is merged, the current API for mode=a (if anyone is really using it) should not break.
There was a problem hiding this comment.
I support @yuxuanzhuang's approach here, keep it simple to make sure we can merge quickly.
Fixes #3861
Changes made in this Pull Request:
appendfor appending trajectory to WriterBasewriteis not overwritten by Writers (they only modified_write_next_frame)TODO:
PR Checklist