-
Notifications
You must be signed in to change notification settings - Fork 1
[DNM] Refactor reader.py #77
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
base: main
Are you sure you want to change the base?
Conversation
…e runners for the matrix
for more information, see https://pre-commit.ci
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #77 +/- ##
==========================================
- Coverage 88.16% 87.00% -1.17%
==========================================
Files 7 7
Lines 338 354 +16
==========================================
+ Hits 298 308 +10
- Misses 40 46 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # The MDAnalysis trajectory "dt" is the iteration dt | ||
| # multiplied by the number of iterations between frames. | ||
| self._dt = _determine_iteration_dt(self._dataset) * np.diff(self._frames)[0] | ||
| self._frame_index = -1 |
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 wouldn't do this, let _read_frame set the frame_index to 0. Otherwise you're setting the variable to a value only for it to be reset two instructions later (it's a bit of a code smell).
| Path to the .nc file or an open Dataset. | ||
| index : int | ||
| Index of the state or replica to extract. May be negative. | ||
| view : {"state", "replica"}, default "state" |
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.
view is a bit unclear, I would maybe call it something like index_style or index_method? That way it's clear you're talking about how it's being indexed.
| else: | ||
| size = self._dataset.dimensions["replica"].size | ||
|
|
||
| self._index = index % size |
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 would call this multistate_index or something like that, otherwise it gets very confusing with frame_index, etc...
No description provided.