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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GROMOS11 Reader #4292
GROMOS11 Reader #4292
Changes from 9 commits
7dfc569
446c2a0
a95c9fa
dfdb45f
e1867ba
c2ddedb
b842ba9
4ef672b
cfd6b39
ef76854
81ba585
8186650
859b49c
3bb16b5
3b68f20
7796846
9f123ed
7dd51fe
960f3cd
bcbbefa
93bc53f
64fac25
61a0561
c2197b1
680ae2f
abed227
ef7238b
bc69032
6eaaeac
604bd53
780a46a
fddb54b
827eecb
9fca76a
41c1a59
487ca3f
b313696
92084e0
e612526
c4374cd
f0982a9
a88b3bf
40a1ad2
8b5beb5
ede2202
7c624ac
7ab5d0a
1691782
3e666a8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 82 in package/MDAnalysis/coordinates/TRC.py
Codecov / codecov/patch
package/MDAnalysis/coordinates/TRC.py#L81-L82
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test for the behavior when 0 is returned? Does this trigger when e.g. reading an empty file?
Does it make sense to return 0 instead of failing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should fail, as you need the number of atoms to
init
timestep in a way that makes sense.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this does not make too much sense here. I did some tests and if the trajectory file is missing or empty other errors are risen first.
Empty file:
No file:
The program then continues to run and crashes in line 395 of the TRC reader:
Something is going on there but that may be investigated on a general and not a per-reader level. The first error was thrown from a non-compressed file.
I will remove the try/except mentioned here, but I will add a check whether there is any trajectory data in the file. For example, that check would trigger if the trajectory file exists but only contains a single space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a test that will throw an error if there is for example only a single space (so no trajectory info) in a file. If the file is completely empty, other errors (see above) will be risen before.
Check warning on line 94 in package/MDAnalysis/coordinates/TRC.py
Codecov / codecov/patch
package/MDAnalysis/coordinates/TRC.py#L93-L94
Check warning on line 120 in package/MDAnalysis/coordinates/TRC.py
Codecov / codecov/patch
package/MDAnalysis/coordinates/TRC.py#L120
Check warning on line 231 in package/MDAnalysis/coordinates/TRC.py
Codecov / codecov/patch
package/MDAnalysis/coordinates/TRC.py#L231
Check warning on line 238 in package/MDAnalysis/coordinates/TRC.py
Codecov / codecov/patch
package/MDAnalysis/coordinates/TRC.py#L237-L238
Check warning on line 268 in package/MDAnalysis/coordinates/TRC.py
Codecov / codecov/patch
package/MDAnalysis/coordinates/TRC.py#L267-L268
Check warning on line 286 in package/MDAnalysis/coordinates/TRC.py
Codecov / codecov/patch
package/MDAnalysis/coordinates/TRC.py#L286
Check warning on line 300 in package/MDAnalysis/coordinates/TRC.py
Codecov / codecov/patch
package/MDAnalysis/coordinates/TRC.py#L300