Added get_connections #3160
Added get_connections #3160
Conversation
|
Hello @lilyminium! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-04-04 13:46:20 UTC |
Codecov Report
@@ Coverage Diff @@
## master #3160 +/- ##
==========================================
+ Coverage 90.98% 91.79% +0.80%
==========================================
Files 162 167 +5
Lines 22197 22725 +528
Branches 3201 3206 +5
==========================================
+ Hits 20196 20860 +664
- Misses 1378 1781 +403
+ Partials 623 84 -539
Continue to review full report at Codecov.
|
| return mda.Universe(TPR) | ||
|
|
||
|
|
||
| class TestGetConnections(object): |
There was a problem hiding this comment.
Do you mean those specific files and selections, or do you mean using the .bonds and .dihedrals attributes? The latter will have to wait until 2.0
c7a9420 to
3244844
Compare
| .. versionadded:: 1.1.0 | ||
| """ | ||
| # AtomGroup has handy error messages for missing attributes | ||
| ugroup = getattr(self.universe.atoms, typename) |
There was a problem hiding this comment.
So we're not using atomgroup_intersection here because this method can handle not just things in a TopologyGroup?
There was a problem hiding this comment.
I think it was more to avoid having to distinguish between Atoms and AtomGroups (etc) with ix_array, as atomgroup_intersection only accepts AtomGroups
There was a problem hiding this comment.
I just realised that Residue.bonds is not existing functionality, so I added tests.
| bond_idx, types, guessed, order = np.hsplit( | ||
| np.array(sorted(unique_bonds), dtype=object), 4) | ||
| unique_bonds = np.array(sorted(unique_bonds), dtype=object) | ||
| if not outside: |
There was a problem hiding this comment.
Why use a not here?
if outside:
<deprecation code>
else:
<np.all>
It makes it more readable imo
There was a problem hiding this comment.
I started with strict=False 😅
This will make it easier to add to 2.0, though, where we will not have the deprecation warning.
a17359e to
2ea7bbe
Compare
2ea7bbe to
c2716b9
Compare
|
@IAlibay I forget where you asked me this, but the reason |
Relates to #1264
Relates to #2821
Changes made in this Pull Request:
get_connectionsmethod for Components and Groups to control which connections get returnedoutsidekeyword toget_atomsargument of the Connection topology attributes, to easily toggle the behaviour of Groups.bonds, Groups.angles, etc. This is set toTrueto keep status quo for 1.1.0. It will be set to False in 2.0.0.PR Checklist