Add intra_connections (intra_bonds, intra_angles, etc) by lilyminium · Pull Request #3200 · MDAnalysis/mdanalysis

lilyminium · 2021-04-02T03:16:11Z

Fixes #1264
Fixes #2821

Changes made in this Pull Request:

~~After PR Added get_connections #3160, sets the default get_atoms to only return connections where all atoms are within the queried AtomGroup (i.e. outside=False)~~
~~However, it still returns all connections involving the atom in Atom.bonds/angles/dihedrals etc (i.e. outside=True)~~
~~I would expect Atom.bonds to include all the bonds involving that Atom, but AtomGroup to only include the bonds within the group.~~
~~Also changes various parsers and writers to use get_connections instead of .bonds etc to get the previous data~~
~~Changes a bunch of tests to pass~~
Added intra_bonds, etc. by way of metaclass jiggling.

PR Checklist

Tests?
Docs?
CHANGELOG updated?
Issue raised/referenced?

pep8speaks · 2021-04-02T03:16:24Z

Hello @lilyminium! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file testsuite/MDAnalysisTests/core/test_groups.py:

Line 1474:1: E302 expected 2 blank lines, found 1

Comment last updated at 2021-05-02 17:03:39 UTC

codecov · 2021-04-02T03:38:50Z

Codecov Report

Merging #3200 (c7997b5) into develop (fe22dc3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #3200   +/-   ##
========================================
  Coverage    92.85%   92.86%           
========================================
  Files          170      170           
  Lines        22840    22865   +25     
  Branches      3240     3243    +3     
========================================
+ Hits         21208    21233   +25     
  Misses        1584     1584           
  Partials        48       48

Impacted Files	Coverage Δ
package/MDAnalysis/coordinates/MOL2.py	`97.69% <100.00%> (ø)`
package/MDAnalysis/coordinates/ParmEd.py	`93.18% <100.00%> (ø)`
package/MDAnalysis/core/groups.py	`98.47% <100.00%> (+0.01%)`	⬆️
package/MDAnalysis/core/topologyattrs.py	`96.72% <100.00%> (+0.03%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe22dc3...c7997b5. Read the comment docs.

…develop

jbarnoud

The good thing with a good self-contained and well scoped PR is that I can nitpick. So here is my nitpicking.

package/MDAnalysis/core/groups.py

jbarnoud · 2021-04-28T09:52:59Z

package/MDAnalysis/core/groups.py

+        ugroup = getattr(self.universe.atoms, typename)
+        if not len(ugroup):
+            return ugroup
+        func = np.any if outside else np.all


I'd prefer a more explicit name for that. logic_comparator maybe?

I see the conversation marked as resolved, but the variable is still named func.

jbarnoud · 2021-04-28T10:01:22Z

package/MDAnalysis/core/topologyattrs.py


    def get_atoms(self, ag):
+        """
+        Get subset for atoms.


It is good to add a docstring, but I am not sure this one helps much: what subset of the atoms?

I hope this new one is more helpful, although I'm not certain users will ever really see it

testsuite/MDAnalysisTests/core/test_groups.py

package/MDAnalysis/core/topologyattrs.py

Co-authored-by: Jonathan Barnoud <jonathan@barnoud.net>

Fixes MDAnalysis#3037 Co-authored-by: Lily Wang <31115101+lilyminium@users.noreply.github.com>

Towards MDAnalysis#2468 ## Work done in this PR * Add aromaticity and Gasteiger charges guessers which work via the RDKIT converter.

Fixes MDAnalysis#3109 ## Work done in this PR * gracefully handle the case where `gcc` toolchain in use on MacOS has been built from source using `clang` by `spack` (so it really is `gcc` in use, not `clang`) ## Notes * we could try to add regression testing, but a few problems: - `using_clang()` is inside `setup.py`, which probably can't be safely imported because it has unguarded statements/ code blocks that run right away - testing build issues is typically tricky with mocking, etc. (though in this case, probably just need to move `using_clang()` somewhere else and then test it against a variety of compiler metadata strings

Fixes MDAnalysis#3031

* Fixes MDAnalysis#2974 * Python 3.9 officially supported * Add Python 3.9 to testing matrix * Adds macOS CI entry, formalises 3.9 support

…lysis into get_connections_develop

jbarnoud

There is one tiny little thing left, really tiny tiny one. I approve the PR, but I'll merge only tomorrow to give you a chance to look at it if you think it is worth changing.

jbarnoud · 2021-04-30T09:15:23Z

package/MDAnalysis/core/groups.py

+        ugroup = getattr(self.universe.atoms, typename)
+        if not len(ugroup):
+            return ugroup
+        func = np.any if outside else np.all


I see the conversation marked as resolved, but the variable is still named func.

jbarnoud · 2021-05-03T18:58:06Z

Sorry, I thought I clicked the button already.

lilyminium added 4 commits April 1, 2021 20:12

added get_connections

8d86a6b

modified tests for atoms.bonds/angles/dihedrals etc

c797056

modified parsers and things to use get_connections or bonds

d07d54b

updated CHANGELOG

1f25fff

pep8

0e3c66c

undo half of PR 3160

20dbe88

IAlibay added this to the 2.0 milestone Apr 6, 2021

lilyminium marked this pull request as draft April 6, 2021 18:49

lilyminium changed the title ~~Set AtomGroup.bonds, angles, dihedrals etc. to only get values *within* AtomGroup~~ Add intra_connections (intra_bonds, intra_angles, etc) Apr 7, 2021

lilyminium force-pushed the get_connections_develop branch 2 times, most recently from f37d148 to a65edfa Compare April 7, 2021 06:27

add intra stuff

981c22a

lilyminium force-pushed the get_connections_develop branch from a65edfa to 981c22a Compare April 7, 2021 06:30

lilyminium marked this pull request as ready for review April 7, 2021 14:32

IAlibay requested review from jbarnoud and mnmelo April 22, 2021 18:03

Merge remote-tracking branch 'upstream/develop' into get_connections_…

c0f0f52

…develop

lilyminium force-pushed the get_connections_develop branch from 967ebf7 to c0f0f52 Compare April 23, 2021 04:23

jbarnoud reviewed Apr 28, 2021

View reviewed changes

lilyminium and others added 9 commits April 28, 2021 09:54

Update package/MDAnalysis/core/groups.py

829bb9f

Co-authored-by: Jonathan Barnoud <jonathan@barnoud.net>

tighten up base class checking

8b64b46

update docstring

88f8212

suppres warnings

17797e9

Use absolute file paths in ITPParser (MDAnalysis#3108)

3a34048

Fixes MDAnalysis#3037 Co-authored-by: Lily Wang <31115101+lilyminium@users.noreply.github.com>

Adds aromaticity and Gasteiger charges guessers (MDAnalysis#2926)

8fcf354

Towards MDAnalysis#2468 ## Work done in this PR * Add aromaticity and Gasteiger charges guessers which work via the RDKIT converter.

Remove ParmEd Timestep writing "support" (MDAnalysis#3240)

e1c07a6

Fixes MDAnalysis#3031

Adding py3.9 to gh actions CI matrix (MDAnalysis#3245)

cc1e43a

* Fixes MDAnalysis#2974 * Python 3.9 officially supported * Add Python 3.9 to testing matrix * Adds macOS CI entry, formalises 3.9 support

fix changelog

eb0e5c0

lilyminium force-pushed the get_connections_develop branch from 385f956 to eb0e5c0 Compare April 28, 2021 17:44

lilyminium and others added 7 commits April 28, 2021 10:45

Merge branch 'develop' into get_connections_develop

20ba843

special metaclass

e041366

Merge branch 'get_connections_develop' of github.com:lilyminium/mdana…

27637ec

…lysis into get_connections_develop

Merge branch 'develop' into get_connections_develop

cff78e6

move function down

2a06cb7

Merge branch 'get_connections_develop' of github.com:lilyminium/mdana…

cb251d4

…lysis into get_connections_develop

tidy code

0eacf67

jbarnoud approved these changes Apr 30, 2021

View reviewed changes

Merge branch 'develop' into get_connections_develop

c7997b5

jbarnoud merged commit a82113b into MDAnalysis:develop May 3, 2021

IAlibay added the enhancement label Sep 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add intra_connections (intra_bonds, intra_angles, etc)#3200

Add intra_connections (intra_bonds, intra_angles, etc)#3200
jbarnoud merged 26 commits intoMDAnalysis:developfrom
lilyminium:get_connections_develop

lilyminium commented Apr 2, 2021 •

edited

Loading

Uh oh!

pep8speaks commented Apr 2, 2021 •

edited

Loading

Uh oh!

codecov bot commented Apr 2, 2021 •

edited

Loading

Uh oh!

jbarnoud left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jbarnoud Apr 28, 2021

Uh oh!

jbarnoud Apr 30, 2021

Uh oh!

jbarnoud Apr 28, 2021

Uh oh!

lilyminium Apr 28, 2021

Uh oh!

Uh oh!

Uh oh!

jbarnoud left a comment

Uh oh!

jbarnoud Apr 30, 2021

Uh oh!

jbarnoud commented May 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

lilyminium commented Apr 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

pep8speaks commented Apr 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2021-05-02 17:03:39 UTC

Uh oh!

codecov bot commented Apr 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jbarnoud left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jbarnoud Apr 28, 2021

Choose a reason for hiding this comment

Uh oh!

jbarnoud Apr 30, 2021

Choose a reason for hiding this comment

Uh oh!

jbarnoud Apr 28, 2021

Choose a reason for hiding this comment

Uh oh!

lilyminium Apr 28, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jbarnoud left a comment

Choose a reason for hiding this comment

Uh oh!

jbarnoud Apr 30, 2021

Choose a reason for hiding this comment

Uh oh!

jbarnoud commented May 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

lilyminium commented Apr 2, 2021 •

edited

Loading

pep8speaks commented Apr 2, 2021 •

edited

Loading

codecov bot commented Apr 2, 2021 •

edited

Loading