-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update mda dependency #63
Conversation
Since we need to pump our minimum version anyway this won't be a problem anymore.
OK now everything runs with the new version of MDAnalysis. |
This should work now also with 0.15.0 and the new releases |
6f53c4f
to
d8b413c
Compare
Current coverage is 89.48% (diff: 100%)@@ develop #63 diff @@
==========================================
Files 7 7
Lines 357 352 -5
Methods 0 0
Messages 0 0
Branches 58 58
==========================================
- Hits 320 315 -5
Misses 22 22
Partials 15 15
|
The initial missing import problem is now fixed in MDAnalysis development branch. We should still include these fixes. |
@dotsdl ping |
@@ -216,7 +215,11 @@ def _apply_resnums(self): | |||
resnums = None | |||
|
|||
if resnums: | |||
self._treant._universe.residues.set_resnums(np.array(resnums)) | |||
# Compatibility for MDAnalysis pre 0.16.0 |
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 you can just switch to only the .resnums = resnums
branch here, it was supported in 0.14
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.
Eventually we want to get rid of the storage of resnums here. The feature is hidden in MDS at the moment because we definitely won't be supporting it in the future, but it was really useful and important to work I was currently doing. With the new topology system in MDA and usage of persistent topologies we can completely eliminate this soon.
Thanks for this, @kain88-de. We keep it for now.
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.
Awesome. Looks great @kain88-de! Merging.
@@ -216,7 +215,11 @@ def _apply_resnums(self): | |||
resnums = None | |||
|
|||
if resnums: | |||
self._treant._universe.residues.set_resnums(np.array(resnums)) | |||
# Compatibility for MDAnalysis pre 0.16.0 |
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.
Eventually we want to get rid of the storage of resnums here. The feature is hidden in MDS at the moment because we definitely won't be supporting it in the future, but it was really useful and important to work I was currently doing. With the new topology system in MDA and usage of persistent topologies we can completely eliminate this soon.
Thanks for this, @kain88-de. We keep it for now.
This includes various fixes to be usable with the new MDAnalysis version. kwargs can't be stored right now because of an old bug that has reappeared in MDAnalysis.