From 14bd0d4da4bca69e205a45be8f5ea2276c0194ff Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Sat, 11 Nov 2017 12:31:14 +0000
Subject: [PATCH 01/23] Work on Issue #1092

Currently failing
---
 package/MDAnalysis/core/groups.py             | 80 +++++++++++++------
 package/MDAnalysis/core/topologyattrs.py      | 56 ++++++-------
 package/MDAnalysis/core/universe.py           | 35 ++++----
 testsuite/MDAnalysisTests/core/test_groups.py | 25 ++++++
 4 files changed, 125 insertions(+), 71 deletions(-)

diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py
index c5c5931cda3..7afdb5c4735 100644
--- a/package/MDAnalysis/core/groups.py
+++ b/package/MDAnalysis/core/groups.py
@@ -153,15 +153,21 @@ def make_classes():
 
     # The 'GBase' middle man is needed so that a single topologyattr
     #  patching applies automatically to all groups.
-    GBase = bases[GroupBase] = _TopologyAttrContainer._subclass()
+    GBase = bases[GroupBase] = _TopologyAttrContainer._subclass(singular=False)
+    GBase._SETATTR_WHITELIST = {  # list of Group attributes we can set
+        'positions', 'velocities', 'forces', 'dimensions',
+        'atoms', 'residue', 'residues', 'segment', 'segments',
+    }
     for cls in groups:
-        bases[cls] = GBase._subclass()
-
-    # In the current Group-centered topology scheme no attributes apply only
-    #  to ComponentBase, so no need to have a 'CB' middle man.
-    #CBase = _TopologyAttrContainer(singular=True)
+        bases[cls] = GBase._subclass(singular=False)
+    # CBase for patching all components
+    CBase = bases[ComponentBase] = _TopologyAttrContainer._subclass(singular=True)
+    CBase._SETATTR_WHITELIST = {
+        'position', 'velocity', 'force', 'dimensions',
+        'atoms', 'residue', 'residues', 'segment', 'segments',
+    }
     for cls in components:
-        bases[cls] = _TopologyAttrContainer._subclass(singular=True)
+        bases[cls] = CBase._subclass(singular=True)
 
     # Initializes the class cache.
     for cls in groups + components:
@@ -184,10 +190,8 @@ class _TopologyAttrContainer(object):
       The mixed subclasses become the final container classes specific to each
       :class:`Universe`.
     """
-    _singular = False
-
     @classmethod
-    def _subclass(cls, singular=None):
+    def _subclass(cls, singular):
         """Factory method returning :class:`_TopologyAttrContainer` subclasses.
 
         Parameters
@@ -201,10 +205,7 @@ def _subclass(cls, singular=None):
         type
             A subclass of :class:`_TopologyAttrContainer`, with the same name.
         """
-        if singular is not None:
-            return type(cls.__name__, (cls,), {'_singular': bool(singular)})
-        else:
-            return type(cls.__name__, (cls,), {})
+        return type(cls.__name__, (cls,), {'_singular': bool(singular)})
 
     @classmethod
     def _mix(cls, other):
@@ -256,6 +257,12 @@ def setter(self, values):
             setattr(cls, attr.attrname,
                     property(getter, setter, None, attr.groupdoc))
 
+    @classmethod
+    def _whitelist(cls, attr):
+        """Allow an attribute to be set in Groups"""
+        cls._SETATTR_WHITELIST.add(attr.attrname)
+        cls._SETATTR_WHITELIST.add(attr.singular)
+
 
 class _MutableBase(object):
     """
@@ -428,8 +435,22 @@ def __init__(self, *args):
     def __hash__(self):
         return hash((self._u, self.__class__, tuple(self.ix.tolist())))
 
+    def __setattr__(self, attr, value):
+        # `ag.this = 42` calls setattr(ag, 'this', 42)
+        # we scan 'this' to see if it is either 'private'
+        # or a known attribute (WHITELIST)
+        if (not attr.startswith('_') and
+            type(self) in (self.universe._classes[AtomGroup],
+                           self.universe._classes[ResidueGroup],
+                           self.universe._classes[SegmentGroup])
+            and not attr in self._SETATTR_WHITELIST):
+            raise AttributeError("Cannot set arbitrary attributes to a Group")
+        # if it is, we allow the setattr to proceed by deferring to the super
+        # behaviour (ie do it)
+        super(GroupBase, self).__setattr__(attr, value)
+
     def __len__(self):
-        return len(self._ix)
+        return len(self.ix)
 
     def __getitem__(self, item):
         # supports
@@ -1844,9 +1865,6 @@ def select_atoms(self, sel, *othersel, **selgroups):
                 which is often the case with high-resolution crystal structures
                 e.g. `resid 4 and resname ALA and altloc B` selects only the
                 atoms of ALA-4 that have an altloc B record.
-            moltype *molecule-type*
-                select by molecule type, e.g. ``moltype Protein_A``. At the
-                moment, only the TPR format defines the molecule type.
 
         **Boolean**
 
@@ -2437,6 +2455,20 @@ def __init__(self, ix, u):
         self._ix = ix
         self._u = u
 
+    def __setattr__(self, attr, value):
+        if (not attr.startswith('_') and
+            type(self) in (self.universe._classes[Atom],
+                           self.universe._classes[Residue],
+                           self.universe._classes[Segment]) and
+            not attr in self._SETATTR_WHITELIST):
+            raise AttributeError(
+                "Cannot set arbitrary attributes to a Component")
+        super(ComponentBase, self).__setattr__(attr, value)
+
+    def __repr__(self):
+        return ("<{} {}>"
+                "".format(self.level.name.capitalize(), self.ix))
+
     def __lt__(self, other):
         if self.level != other.level:
             raise TypeError("Can't compare different level objects")
@@ -2780,7 +2812,7 @@ def __init__(self, base_group, selections, strings):
         # its check, no self.attribute access can be made before this line
         self._u = base_group.universe
         self._selections = selections
-        self.selection_strings = strings
+        self._selection_strings = strings
         self._base_group = base_group
         self._lastupdate = None
         self._derived_class = base_group._derived_class
@@ -2858,7 +2890,7 @@ def _ensure_updated(self):
     def __getattribute__(self, name):
         # ALL attribute access goes through here
         # If the requested attribute isn't in the shortcut list, update ourselves
-        if not name in _UAG_SHORTCUT_ATTRS:
+        if not (name.startswith('_') or name in _UAG_SHORTCUT_ATTRS):
             self._ensure_updated()
         # Going via object.__getattribute__ then bypasses this check stage
         return object.__getattribute__(self, name)
@@ -2869,13 +2901,13 @@ def __reduce__(self):
         # - recreate UAG as created through select_atoms (basegroup and selstrs)
         # even if base_group is a UAG this will work through recursion
         return (_unpickle_uag,
-                (self._base_group.__reduce__(), self._selections, self.selection_strings))
+                (self._base_group.__reduce__(), self._selections, self._selection_strings))
 
     def __repr__(self):
         basestr = super(UpdatingAtomGroup, self).__repr__()
-        if not self.selection_strings:
+        if not self._selection_strings:
             return basestr
-        sels = "'{}'".format("' + '".join(self.selection_strings))
+        sels = "'{}'".format("' + '".join(self._selection_strings))
         # Cheap comparison. Might fail for corner cases but this is
         # mostly cosmetic.
         if self._base_group is self.universe.atoms:
@@ -2884,7 +2916,7 @@ def __repr__(self):
             basegrp = "another AtomGroup."
         # With a shorthand to conditionally append the 's' in 'selections'.
         return "{}, with selection{} {} on {}>".format(basestr[:-1],
-                    "s"[len(self.selection_strings)==1:], sels, basegrp)
+                    "s"[len(self._selection_strings)==1:], sels, basegrp)
 
 # Define relationships between these classes
 # with Level objects
diff --git a/package/MDAnalysis/core/topologyattrs.py b/package/MDAnalysis/core/topologyattrs.py
index 9b29f3533bb..51dc6533805 100644
--- a/package/MDAnalysis/core/topologyattrs.py
+++ b/package/MDAnalysis/core/topologyattrs.py
@@ -187,6 +187,8 @@ class TopologyAttr(object):
     singular = 'topologyattr'
     per_object = None  # ie Resids per_object = 'residue'
     top = None  # pointer to Topology object
+    transplants = defaultdict(list)
+    target_classes = []
 
     groupdoc = None
     singledoc = None
@@ -271,13 +273,13 @@ def set_atoms(self, ag, values):
         raise AttributeError("Atom indices are fixed; they cannot be reset")
 
     def get_atoms(self, ag):
-        return ag._ix
+        return ag.ix
 
     def get_residues(self, rg):
-        return list(self.top.tt.residues2atoms_2d(rg._ix))
+        return list(self.top.tt.residues2atoms_2d(rg.ix))
 
     def get_segments(self, sg):
-        return list(self.top.tt.segments2atoms_2d(sg._ix))
+        return list(self.top.tt.segments2atoms_2d(sg.ix))
 
 
 class Resindices(TopologyAttr):
@@ -300,16 +302,16 @@ def __init__(self):
         self._guessed = False
 
     def get_atoms(self, ag):
-        return self.top.tt.atoms2residues(ag._ix)
+        return self.top.tt.atoms2residues(ag.ix)
 
     def get_residues(self, rg):
-        return rg._ix
+        return rg.ix
 
     def set_residues(self, rg, values):
         raise AttributeError("Residue indices are fixed; they cannot be reset")
 
     def get_segments(self, sg):
-        return list(self.top.tt.segments2residues_2d(sg._ix))
+        return list(self.top.tt.segments2residues_2d(sg.ix))
 
 
 class Segindices(TopologyAttr):
@@ -333,13 +335,13 @@ def __init__(self):
         self._guessed = False
 
     def get_atoms(self, ag):
-        return self.top.tt.atoms2segments(ag._ix)
+        return self.top.tt.atoms2segments(ag.ix)
 
     def get_residues(self, rg):
-        return self.top.tt.residues2segments(rg._ix)
+        return self.top.tt.residues2segments(rg.ix)
 
     def get_segments(self, sg):
-        return sg._ix
+        return sg.ix
 
     def set_segments(self, sg, values):
         raise AttributeError("Segment indices are fixed; they cannot be reset")
@@ -356,11 +358,11 @@ class AtomAttr(TopologyAttr):
     target_classes = [Atom]
 
     def get_atoms(self, ag):
-        return self.values[ag._ix]
+        return self.values[ag.ix]
 
     @_check_length
     def set_atoms(self, ag, values):
-        self.values[ag._ix] = values
+        self.values[ag.ix] = values
 
     def get_residues(self, rg):
         """By default, the values for each atom present in the set of residues
@@ -368,7 +370,7 @@ def get_residues(self, rg):
         attributes.
 
         """
-        aixs = self.top.tt.residues2atoms_2d(rg._ix)
+        aixs = self.top.tt.residues2atoms_2d(rg.ix)
         return [self.values[aix] for aix in aixs]
 
     def set_residues(self, rg, values):
@@ -380,7 +382,7 @@ def get_segments(self, sg):
         attributes.
 
         """
-        aixs = self.top.tt.segments2atoms_2d(sg._ix)
+        aixs = self.top.tt.segments2atoms_2d(sg.ix)
         return [self.values[aix] for aix in aixs]
 
     def set_segments(self, sg, values):
@@ -621,7 +623,7 @@ def __init__(self, values, guessed=False):
         self._guessed = guessed
 
     def get_residues(self, rg):
-        resatoms = self.top.tt.residues2atoms_2d(rg._ix)
+        resatoms = self.top.tt.residues2atoms_2d(rg.ix)
 
         if isinstance(rg._ix, numbers.Integral):
             # for a single residue
@@ -635,7 +637,7 @@ def get_residues(self, rg):
         return masses
 
     def get_segments(self, sg):
-        segatoms = self.top.tt.segments2atoms_2d(sg._ix)
+        segatoms = self.top.tt.segments2atoms_2d(sg.ix)
 
         if isinstance(sg._ix, numbers.Integral):
             # for a single segment
@@ -947,7 +949,7 @@ class Charges(AtomAttr):
     transplants = defaultdict(list)
 
     def get_residues(self, rg):
-        resatoms = self.top.tt.residues2atoms_2d(rg._ix)
+        resatoms = self.top.tt.residues2atoms_2d(rg.ix)
 
         if isinstance(rg._ix, numbers.Integral):
             charges = self.values[resatoms].sum()
@@ -959,7 +961,7 @@ def get_residues(self, rg):
         return charges
 
     def get_segments(self, sg):
-        segatoms = self.top.tt.segments2atoms_2d(sg._ix)
+        segatoms = self.top.tt.segments2atoms_2d(sg.ix)
 
         if isinstance(sg._ix, numbers.Integral):
             # for a single segment
@@ -1020,18 +1022,18 @@ class ResidueAttr(TopologyAttr):
     per_object = 'residue'
 
     def get_atoms(self, ag):
-        rix = self.top.tt.atoms2residues(ag._ix)
+        rix = self.top.tt.atoms2residues(ag.ix)
         return self.values[rix]
 
     def set_atoms(self, ag, values):
         raise _wronglevel_error(self, ag)
 
     def get_residues(self, rg):
-        return self.values[rg._ix]
+        return self.values[rg.ix]
 
     @_check_length
     def set_residues(self, rg, values):
-        self.values[rg._ix] = values
+        self.values[rg.ix] = values
 
     def get_segments(self, sg):
         """By default, the values for each residue present in the set of
@@ -1039,7 +1041,7 @@ def get_segments(self, sg):
         in child attributes.
 
         """
-        rixs = self.top.tt.segments2residues_2d(sg._ix)
+        rixs = self.top.tt.segments2residues_2d(sg.ix)
         return [self.values[rix] for rix in rixs]
 
     def set_segments(self, sg, values):
@@ -1247,25 +1249,25 @@ class SegmentAttr(TopologyAttr):
     per_object = 'segment'
 
     def get_atoms(self, ag):
-        six = self.top.tt.atoms2segments(ag._ix)
+        six = self.top.tt.atoms2segments(ag.ix)
         return self.values[six]
 
     def set_atoms(self, ag, values):
         raise _wronglevel_error(self, ag)
 
     def get_residues(self, rg):
-        six = self.top.tt.residues2segments(rg._ix)
+        six = self.top.tt.residues2segments(rg.ix)
         return self.values[six]
 
     def set_residues(self, rg, values):
         raise _wronglevel_error(self, rg)
 
     def get_segments(self, sg):
-        return self.values[sg._ix]
+        return self.values[sg.ix]
 
     @_check_length
     def set_segments(self, sg, values):
-        self.values[sg._ix] = values
+        self.values[sg.ix] = values
 
 
 # TODO: update docs to property doc
@@ -1373,10 +1375,10 @@ def set_atoms(self, ag):
     def get_atoms(self, ag):
         try:
             unique_bonds = set(itertools.chain(
-                *[self._bondDict[a] for a in ag._ix]))
+                *[self._bondDict[a] for a in ag.ix]))
         except TypeError:
             # maybe we got passed an Atom
-            unique_bonds = self._bondDict[ag._ix]
+            unique_bonds = self._bondDict[ag.ix]
         bond_idx, types, guessed, order = np.hsplit(
             np.array(sorted(unique_bonds)), 4)
         bond_idx = np.array(bond_idx.ravel().tolist(), dtype=np.int32)
diff --git a/package/MDAnalysis/core/universe.py b/package/MDAnalysis/core/universe.py
index 67775c24034..ddabf164ca7 100644
--- a/package/MDAnalysis/core/universe.py
+++ b/package/MDAnalysis/core/universe.py
@@ -100,7 +100,8 @@
 from ..lib.util import cached, NamedStream, isstream
 from . import groups
 from ._get_readers import get_reader_for, get_parser_for
-from .groups import (GroupBase, Atom, Residue, Segment,
+from .groups import (ComponentBase, GroupBase,
+                     Atom, Residue, Segment,
                      AtomGroup, ResidueGroup, SegmentGroup)
 from .topology import Topology
 from .topologyattrs import AtomAttr, ResidueAttr, SegmentAttr
@@ -698,27 +699,21 @@ def _process_attr(self, attr):
                                  m=len(attr)))
 
         self._class_bases[GroupBase]._add_prop(attr)
+        self._class_bases[GroupBase]._whitelist(attr)
+        self._class_bases[ComponentBase]._whitelist(attr)
 
         for cls in attr.target_classes:
-            try:
-                self._class_bases[cls]._add_prop(attr)
-            except (KeyError, AttributeError):
-                pass
-
-        try:
-            transplants = attr.transplants
-        except AttributeError:
-            # not every Attribute will have a transplant dict
-            pass
-        else:
-            # Group transplants
-            for cls in (Atom, Residue, Segment, GroupBase,
-                        AtomGroup, ResidueGroup, SegmentGroup):
-                for funcname, meth in transplants[cls]:
-                    setattr(self._class_bases[cls], funcname, meth)
-            # Universe transplants
-            for funcname, meth in transplants['Universe']:
-                setattr(self.__class__, funcname, meth)
+            self._class_bases[cls]._add_prop(attr)
+
+        # TODO: Try and shove this into cls._add_prop
+        # Group transplants
+        for cls in (Atom, Residue, Segment, GroupBase,
+                    AtomGroup, ResidueGroup, SegmentGroup):
+            for funcname, meth in attr.transplants[cls]:
+                setattr(self._class_bases[cls], funcname, meth)
+        # Universe transplants
+        for funcname, meth in attr.transplants['Universe']:
+            setattr(self.__class__, funcname, meth)
 
     def add_Residue(self, segment=None, **attrs):
         """Add a new Residue to this Universe
diff --git a/testsuite/MDAnalysisTests/core/test_groups.py b/testsuite/MDAnalysisTests/core/test_groups.py
index 1afbe6aa61b..b9812e79aaf 100644
--- a/testsuite/MDAnalysisTests/core/test_groups.py
+++ b/testsuite/MDAnalysisTests/core/test_groups.py
@@ -1016,3 +1016,28 @@ def test_SegmentGroup_warn_getattr(self, u):
     def test_SegmentGroup_nowarn_getitem(self, u):
         with no_deprecated_call():
             u.segments[0]
+
+
+class TestAttributeSetting(object):
+    @pytest.mark.parametrize('groupname', ['atoms', 'residues', 'segments'])
+    def test_setting_group_fail(self, groupname):
+        u = make_Universe()
+
+        group = getattr(u, groupname)
+        with pytest.raises(AttributeError):
+            group.this = 'that'
+
+    @pytest.mark.parametrize('groupname', ['atoms', 'residues', 'segments'])
+    def test_setting_component_fails(self, groupname):
+        u = make_Universe()
+        component = getattr(u, groupname)[0]
+
+        with pytest.raises(AttributeError):
+            component.this = 'that'
+
+    def test_atomgroup_resid(self):
+        # this should fail as you can't set the resid of an AG
+        u = make_Universe(('resids'))
+
+        with pytest.raises(AttributeError):
+            u.atoms.resid = 24

From 224ee3b50d047fbda795aaf12153a302e93ca9c2 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Sun, 12 Nov 2017 15:54:56 +0000
Subject: [PATCH 02/23] More tests for attribute setting

---
 testsuite/MDAnalysisTests/core/test_groups.py | 71 ++++++++++++++++---
 1 file changed, 60 insertions(+), 11 deletions(-)

diff --git a/testsuite/MDAnalysisTests/core/test_groups.py b/testsuite/MDAnalysisTests/core/test_groups.py
index b9812e79aaf..09c4628574b 100644
--- a/testsuite/MDAnalysisTests/core/test_groups.py
+++ b/testsuite/MDAnalysisTests/core/test_groups.py
@@ -1018,26 +1018,75 @@ def test_SegmentGroup_nowarn_getitem(self, u):
             u.segments[0]
 
 
+@pytest.fixture()
+def attr_universe():
+    return make_Universe(('names', 'resids', 'segids'))
+            
 class TestAttributeSetting(object):
     @pytest.mark.parametrize('groupname', ['atoms', 'residues', 'segments'])
-    def test_setting_group_fail(self, groupname):
-        u = make_Universe()
-
-        group = getattr(u, groupname)
+    def test_setting_group_fail(self, attr_universe, groupname):
+        group = getattr(attr_universe, groupname)
+ 
         with pytest.raises(AttributeError):
             group.this = 'that'
 
     @pytest.mark.parametrize('groupname', ['atoms', 'residues', 'segments'])
-    def test_setting_component_fails(self, groupname):
-        u = make_Universe()
-        component = getattr(u, groupname)[0]
+    def test_setting_component_fails(self, attr_universe, groupname):
+        component = getattr(attr_universe, groupname)[0]
 
         with pytest.raises(AttributeError):
             component.this = 'that'
 
-    def test_atomgroup_resid(self):
-        # this should fail as you can't set the resid of an AG
-        u = make_Universe(('resids'))
+    @pytest.mark.parametrize('attr', ['name', 'resid', 'segid'])
+    @pytest.mark.parametrize('groupname', ['atoms', 'residues', 'segments'])    
+    def test_group_set_singular(self, attr_universe, attr, groupname):
+        # this should fail as you can't set the 'name' of a 'ResidueGroup'
+        group = getattr(attr_universe, groupname)
+        with pytest.raises(AttributeError):
+            setattr(group, attr, 24)
+
+    def test_atom_set_name(self, attr_universe):
+        attr_universe.atoms[0].name = 'this'
+        assert attr_universe.atoms[0].name == 'this'
+
+    def test_atom_set_resid(self, attr_universe):
+        with pytest.raises(NotImplementedError):
+            attr_universe.atoms[0].resid = 24
+
+    def test_atom_set_segid(self, attr_universe):
+        with pytest.raises(NotImplementedError):
+            attr_universe.atoms[0].segid = 'this'
 
+    def test_residue_set_name(self, attr_universe):
+        with pytest.raises(AttributeError):
+            attr_universe.residues[0].name = 'this'
+
+    def test_residue_set_resid(self, attr_universe):
+        attr_universe.residues[0].resid = 24
+        assert attr_universe.residues[0].resid == 24
+
+    def test_residue_set_segid(self, attr_universe):
+        with pytest.raises(NotImplementedError):
+            attr_universe.residues[0].segid = 'this'
+
+    def test_segment_set_name(self, attr_universe):
+        with pytest.raises(AttributeError):
+            attr_universe.segments[0].name = 'this'
+
+    def test_segment_set_resid(self, attr_universe):
+        with pytest.raises(AttributeError):
+            attr_universe.segments[0].resid = 24
+
+    def test_segment_set_segid(self, attr_universe):
+        attr_universe.segments[0].segid = 'this'
+        assert attr_universe.segments[0].segid == 'this'
+            
+    @pytest.mark.parametrize('attr', ['names', 'resids', 'segids'])
+    @pytest.mark.parametrize('groupname', ['atoms', 'residues', 'segments'])    
+    def test_component_set_plural(self, attr, groupname):
+        # this should fail as you can't set the 'Names' of an 'Atom'
+        u = make_Universe(('names', 'resids', 'segids'))
+        group = getattr(u, groupname)
+        comp = group[0]
         with pytest.raises(AttributeError):
-            u.atoms.resid = 24
+            setattr(comp, attr, 24)

From 2c96fcaf5ef5678442a34752859f5a6cc38a7854 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Mon, 20 Nov 2017 21:21:23 +0000
Subject: [PATCH 03/23] Fixed WHITELIST for singular and plural attributes

---
 package/MDAnalysis/core/groups.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py
index 7afdb5c4735..a4023879363 100644
--- a/package/MDAnalysis/core/groups.py
+++ b/package/MDAnalysis/core/groups.py
@@ -260,8 +260,10 @@ def setter(self, values):
     @classmethod
     def _whitelist(cls, attr):
         """Allow an attribute to be set in Groups"""
-        cls._SETATTR_WHITELIST.add(attr.attrname)
-        cls._SETATTR_WHITELIST.add(attr.singular)
+        if cls._singular:
+            cls._SETATTR_WHITELIST.add(attr.singular)
+        else:
+            cls._SETATTR_WHITELIST.add(attr.attrname)
 
 
 class _MutableBase(object):

From a71ed7084d6519cc766dca42917c3549511d9ca7 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Tue, 21 Nov 2017 19:21:04 +0000
Subject: [PATCH 04/23] Changed to a _SETATTR_WHITELIST per class, (not shared)

---
 package/MDAnalysis/core/groups.py        | 25 ++++++------
 package/MDAnalysis/core/topologyattrs.py | 51 +++++++++++-------------
 package/MDAnalysis/core/universe.py      |  4 --
 3 files changed, 37 insertions(+), 43 deletions(-)

diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py
index a4023879363..3491fd774cf 100644
--- a/package/MDAnalysis/core/groups.py
+++ b/package/MDAnalysis/core/groups.py
@@ -154,18 +154,10 @@ def make_classes():
     # The 'GBase' middle man is needed so that a single topologyattr
     #  patching applies automatically to all groups.
     GBase = bases[GroupBase] = _TopologyAttrContainer._subclass(singular=False)
-    GBase._SETATTR_WHITELIST = {  # list of Group attributes we can set
-        'positions', 'velocities', 'forces', 'dimensions',
-        'atoms', 'residue', 'residues', 'segment', 'segments',
-    }
     for cls in groups:
         bases[cls] = GBase._subclass(singular=False)
     # CBase for patching all components
     CBase = bases[ComponentBase] = _TopologyAttrContainer._subclass(singular=True)
-    CBase._SETATTR_WHITELIST = {
-        'position', 'velocity', 'force', 'dimensions',
-        'atoms', 'residue', 'residues', 'segment', 'segments',
-    }
     for cls in components:
         bases[cls] = CBase._subclass(singular=True)
 
@@ -205,7 +197,19 @@ def _subclass(cls, singular):
         type
             A subclass of :class:`_TopologyAttrContainer`, with the same name.
         """
-        return type(cls.__name__, (cls,), {'_singular': bool(singular)})
+        newcls = type(cls.__name__, (cls,), {'_singular': bool(singular)})
+        if singular:
+            newcls._SETATTR_WHITELIST = {
+                'position', 'velocity', 'force', 'dimensions',
+                'atoms', 'residue', 'residues', 'segment', 'segments',
+            }
+        else:
+            newcls._SETATTR_WHITELIST = {
+                'positions', 'velocities', 'forces', 'dimensions',
+                'atoms', 'residue', 'residues', 'segment', 'segments',
+            }
+
+        return newcls
 
     @classmethod
     def _mix(cls, other):
@@ -257,9 +261,6 @@ def setter(self, values):
             setattr(cls, attr.attrname,
                     property(getter, setter, None, attr.groupdoc))
 
-    @classmethod
-    def _whitelist(cls, attr):
-        """Allow an attribute to be set in Groups"""
         if cls._singular:
             cls._SETATTR_WHITELIST.add(attr.singular)
         else:
diff --git a/package/MDAnalysis/core/topologyattrs.py b/package/MDAnalysis/core/topologyattrs.py
index 51dc6533805..30dec6326c2 100644
--- a/package/MDAnalysis/core/topologyattrs.py
+++ b/package/MDAnalysis/core/topologyattrs.py
@@ -166,10 +166,13 @@ def _wronglevel_error(attr, group):
 class TopologyAttr(object):
     """Base class for Topology attributes.
 
-    .. note::   This class is intended to be subclassed, and mostly amounts to
-                a skeleton. The methods here should be present in all
-                :class:`TopologyAttr` child classes, but by default they raise
-                appropriate exceptions.
+    Note
+    ----
+    This class is intended to be subclassed, and mostly amounts to
+    a skeleton. The methods here should be present in all
+    :class:`TopologyAttr` child classes, but by default they raise
+    appropriate exceptions.
+
 
     Attributes
     ----------
@@ -264,7 +267,7 @@ class Atomindices(TopologyAttr):
     """
     attrname = 'indices'
     singular = 'index'
-    target_classes = [Atom]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Atom]
 
     def __init__(self):
         self._guessed = False
@@ -296,7 +299,7 @@ class Resindices(TopologyAttr):
     """
     attrname = 'resindices'
     singular = 'resindex'
-    target_classes = [Atom, Residue]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Atom, Residue]
 
     def __init__(self):
         self._guessed = False
@@ -329,7 +332,8 @@ class Segindices(TopologyAttr):
     """
     attrname = 'segindices'
     singular = 'segindex'
-    target_classes = [Atom, Residue, Segment]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup,
+                      Atom, Residue, Segment]
 
     def __init__(self):
         self._guessed = False
@@ -355,7 +359,7 @@ class AtomAttr(TopologyAttr):
     """
     attrname = 'atomattrs'
     singular = 'atomattr'
-    target_classes = [Atom]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Atom]
 
     def get_atoms(self, ag):
         return self.values[ag.ix]
@@ -605,7 +609,8 @@ class Masses(AtomAttr):
     attrname = 'masses'
     singular = 'mass'
     per_object = 'atom'
-    target_classes = [Atom, Residue, Segment]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup,
+                      Atom, Residue, Segment]
     transplants = defaultdict(list)
 
     groupdoc = """Mass of each component in the Group.
@@ -945,7 +950,8 @@ class Charges(AtomAttr):
     attrname = 'charges'
     singular = 'charge'
     per_object = 'atom'
-    target_classes = [Atom, Residue, Segment]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup,
+                      Atom, Residue, Segment]
     transplants = defaultdict(list)
 
     def get_residues(self, rg):
@@ -1005,20 +1011,10 @@ class AltLocs(AtomAttr):
     per_object = 'atom'
 
 
-# residue attributes
-
 class ResidueAttr(TopologyAttr):
-    """Base class for Topology attributes.
-
-    .. note::   This class is intended to be subclassed, and mostly amounts to
-                a skeleton. The methods here should be present in all
-                :class:`TopologyAttr` child classes, but by default they raise
-                appropriate exceptions.
-
-    """
     attrname = 'residueattrs'
     singular = 'residueattr'
-    target_classes = [Residue]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Residue]
     per_object = 'residue'
 
     def get_atoms(self, ag):
@@ -1053,14 +1049,14 @@ class Resids(ResidueAttr):
     """Residue ID"""
     attrname = 'resids'
     singular = 'resid'
-    target_classes = [Atom, Residue]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Atom, Residue]
 
 
 # TODO: update docs to property doc
 class Resnames(ResidueAttr):
     attrname = 'resnames'
     singular = 'resname'
-    target_classes = [Atom, Residue]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Atom, Residue]
     transplants = defaultdict(list)
 
     def getattr__(residuegroup, resname):
@@ -1218,7 +1214,7 @@ def sequence(self, **kwargs):
 class Resnums(ResidueAttr):
     attrname = 'resnums'
     singular = 'resnum'
-    target_classes = [Atom, Residue]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Atom, Residue]
 
 
 class ICodes(ResidueAttr):
@@ -1234,7 +1230,7 @@ class Moltypes(ResidueAttr):
     """
     attrname = 'moltypes'
     singular = 'moltype'
-    target_classes = [Atom, Residue]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Atom, Residue]
 
 
 # segment attributes
@@ -1245,7 +1241,7 @@ class SegmentAttr(TopologyAttr):
     """
     attrname = 'segmentattrs'
     singular = 'segmentattr'
-    target_classes = [Segment]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Segment]
     per_object = 'segment'
 
     def get_atoms(self, ag):
@@ -1274,7 +1270,8 @@ def set_segments(self, sg, values):
 class Segids(SegmentAttr):
     attrname = 'segids'
     singular = 'segid'
-    target_classes = [Atom, Residue, Segment]
+    target_classes = [AtomGroup, ResidueGroup, SegmentGroup,
+                      Atom, Residue, Segment]
     transplants = defaultdict(list)
 
     def getattr__(segmentgroup, segid):
diff --git a/package/MDAnalysis/core/universe.py b/package/MDAnalysis/core/universe.py
index ddabf164ca7..1af0322b90f 100644
--- a/package/MDAnalysis/core/universe.py
+++ b/package/MDAnalysis/core/universe.py
@@ -698,10 +698,6 @@ def _process_attr(self, attr):
                                  n=n_dict[attr.per_object],
                                  m=len(attr)))
 
-        self._class_bases[GroupBase]._add_prop(attr)
-        self._class_bases[GroupBase]._whitelist(attr)
-        self._class_bases[ComponentBase]._whitelist(attr)
-
         for cls in attr.target_classes:
             self._class_bases[cls]._add_prop(attr)
 

From 5b0cf1eee4b5e2c4a2d0efa6d5f96f0bd57b8e13 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Tue, 21 Nov 2017 19:39:35 +0000
Subject: [PATCH 05/23] Fixup

---
 package/MDAnalysis/core/groups.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py
index 3491fd774cf..d3a9ef663a3 100644
--- a/package/MDAnalysis/core/groups.py
+++ b/package/MDAnalysis/core/groups.py
@@ -1868,6 +1868,9 @@ def select_atoms(self, sel, *othersel, **selgroups):
                 which is often the case with high-resolution crystal structures
                 e.g. `resid 4 and resname ALA and altloc B` selects only the
                 atoms of ALA-4 that have an altloc B record.
+            moltype *molecule-type*
+                select by molecule type, e.g. ``moltype Protein_A``. At the
+                moment, only the TPR format defines the molecule type.
 
         **Boolean**
 

From 4cb43456b3e4d1db5d1284c02c254160143005b9 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Tue, 21 Nov 2017 20:11:23 +0000
Subject: [PATCH 06/23] Simplified setattr across Component and Group

---
 package/MDAnalysis/core/groups.py | 41 ++++++++++---------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py
index d3a9ef663a3..ba26604c1b9 100644
--- a/package/MDAnalysis/core/groups.py
+++ b/package/MDAnalysis/core/groups.py
@@ -266,6 +266,19 @@ def setter(self, values):
         else:
             cls._SETATTR_WHITELIST.add(attr.attrname)
 
+    def __setattr__(self, attr, value):
+        # `ag.this = 42` calls setattr(ag, 'this', 42)
+        # we scan 'this' to see if it is either 'private'
+        # or a known attribute (WHITELIST)
+        if (not attr.startswith('_') and
+            not attr in self._SETATTR_WHITELIST):
+            raise AttributeError(
+                "Cannot set arbitrary attributes to a {}".format(
+                    'Component' if self._singular else 'Group'))
+        # if it is, we allow the setattr to proceed by deferring to the super
+        # behaviour (ie do it)
+        super(_TopologyAttrContainer, self).__setattr__(attr, value)
+
 
 class _MutableBase(object):
     """
@@ -438,20 +451,6 @@ def __init__(self, *args):
     def __hash__(self):
         return hash((self._u, self.__class__, tuple(self.ix.tolist())))
 
-    def __setattr__(self, attr, value):
-        # `ag.this = 42` calls setattr(ag, 'this', 42)
-        # we scan 'this' to see if it is either 'private'
-        # or a known attribute (WHITELIST)
-        if (not attr.startswith('_') and
-            type(self) in (self.universe._classes[AtomGroup],
-                           self.universe._classes[ResidueGroup],
-                           self.universe._classes[SegmentGroup])
-            and not attr in self._SETATTR_WHITELIST):
-            raise AttributeError("Cannot set arbitrary attributes to a Group")
-        # if it is, we allow the setattr to proceed by deferring to the super
-        # behaviour (ie do it)
-        super(GroupBase, self).__setattr__(attr, value)
-
     def __len__(self):
         return len(self.ix)
 
@@ -2461,20 +2460,6 @@ def __init__(self, ix, u):
         self._ix = ix
         self._u = u
 
-    def __setattr__(self, attr, value):
-        if (not attr.startswith('_') and
-            type(self) in (self.universe._classes[Atom],
-                           self.universe._classes[Residue],
-                           self.universe._classes[Segment]) and
-            not attr in self._SETATTR_WHITELIST):
-            raise AttributeError(
-                "Cannot set arbitrary attributes to a Component")
-        super(ComponentBase, self).__setattr__(attr, value)
-
-    def __repr__(self):
-        return ("<{} {}>"
-                "".format(self.level.name.capitalize(), self.ix))
-
     def __lt__(self, other):
         if self.level != other.level:
             raise TypeError("Can't compare different level objects")

From 5444614ad6f32a06130b2e9cbb586e02291f3c7a Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Tue, 21 Nov 2017 21:18:40 +0000
Subject: [PATCH 07/23] Fixed UAGs with restricted attribute setting

---
 package/MDAnalysis/core/groups.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py
index ba26604c1b9..68bf35216e2 100644
--- a/package/MDAnalysis/core/groups.py
+++ b/package/MDAnalysis/core/groups.py
@@ -268,10 +268,9 @@ def setter(self, values):
 
     def __setattr__(self, attr, value):
         # `ag.this = 42` calls setattr(ag, 'this', 42)
-        # we scan 'this' to see if it is either 'private'
-        # or a known attribute (WHITELIST)
-        if (not attr.startswith('_') and
-            not attr in self._SETATTR_WHITELIST):
+        if not (attr.startswith('_') or  # 'private' allowed
+                attr in self._SETATTR_WHITELIST or  # known attributes allowed
+                hasattr(self, attr)):  # preexisting (eg properties) allowed
             raise AttributeError(
                 "Cannot set arbitrary attributes to a {}".format(
                     'Component' if self._singular else 'Group'))

From 5c2f0deabd5533df9ee3049108a87e2191c52976 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Tue, 21 Nov 2017 22:41:17 +0000
Subject: [PATCH 08/23] TopologyAttrs now registered upon definiton

---
 package/MDAnalysis/__init__.py           |  2 ++
 package/MDAnalysis/core/topologyattrs.py | 17 ++++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/package/MDAnalysis/__init__.py b/package/MDAnalysis/__init__.py
index 1eef86fe4cb..fc57737d868 100644
--- a/package/MDAnalysis/__init__.py
+++ b/package/MDAnalysis/__init__.py
@@ -164,6 +164,8 @@
 _MULTIFRAME_WRITERS = {}
 _PARSERS = {}
 _SELECTION_WRITERS = {}
+# Registry of TopologyAttributes
+_TOPOLOGY_ATTRS = {}
 
 # Storing anchor universes for unpickling groups
 import weakref
diff --git a/package/MDAnalysis/core/topologyattrs.py b/package/MDAnalysis/core/topologyattrs.py
index 30dec6326c2..6f871147a76 100644
--- a/package/MDAnalysis/core/topologyattrs.py
+++ b/package/MDAnalysis/core/topologyattrs.py
@@ -31,6 +31,7 @@
 These are usually read by the TopologyParser.
 """
 from __future__ import division, absolute_import
+import six
 from six.moves import zip, range
 
 import Bio.Seq
@@ -54,6 +55,7 @@
 from .groups import (ComponentBase, GroupBase,
                      Atom, Residue, Segment,
                      AtomGroup, ResidueGroup, SegmentGroup)
+from .. import _TOPOLOGY_ATTRS
 
 
 def _check_length(func):
@@ -163,7 +165,20 @@ def _wronglevel_error(attr, group):
     ))
 
 
-class TopologyAttr(object):
+class _TopologyAttrMeta(type):
+    # register TopologyAttrs
+    def __init__(cls, name, bases, classdict):
+        type.__init__(type, name, bases, classdict)
+        for attr in ['attrname', 'singular']:
+            try:
+                attrname = classdict[attr]
+            except KeyError:
+                pass
+            else:
+                _TOPOLOGY_ATTRS[attrname] = cls
+
+
+class TopologyAttr(six.with_metaclass(_TopologyAttrMeta, object)):
     """Base class for Topology attributes.
 
     Note

From 3d7aac3bcce66b5449620044f7f5468de5243b64 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Thu, 23 Nov 2017 00:17:40 +0000
Subject: [PATCH 09/23] Added u.add_TopologyAttribute with string

---
 package/MDAnalysis/core/topologyattrs.py | 88 ++++++++++++++++++++++++
 package/MDAnalysis/core/universe.py      | 35 +++++++++-
 2 files changed, 120 insertions(+), 3 deletions(-)

diff --git a/package/MDAnalysis/core/topologyattrs.py b/package/MDAnalysis/core/topologyattrs.py
index 6f871147a76..fb7c8704810 100644
--- a/package/MDAnalysis/core/topologyattrs.py
+++ b/package/MDAnalysis/core/topologyattrs.py
@@ -215,6 +215,26 @@ def __init__(self, values, guessed=False):
         self.values = values
         self._guessed = guessed
 
+    @staticmethod
+    def _gen_initial_values(n_atoms, n_residues, n_segments):
+        raise NotImplementedError("No default values")
+
+    @classmethod
+    def from_blank(cls, n_atoms=None, n_residues=None, n_segments=None,
+                   values=None):
+        """Create a blank version of this TopologyAttribute
+
+        Parameters
+        ----------
+        n_atoms, n_residues, n_segments : int, optional
+          Size of the TopologyAttribute
+        values : optional
+          Initial values for the TopologyAttribute
+        """
+        if values is None:
+            values = cls._gen_initial_values(n_atoms, n_residues, n_segments)
+        return cls(values)
+
     def __len__(self):
         """Length of the TopologyAttr at its intrinsic level."""
         return len(self.values)
@@ -416,6 +436,10 @@ class Atomids(AtomAttr):
     singular = 'id'
     per_object = 'atom'
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.arange(na) + 1
+
 
 # TODO: update docs to property doc
 class Atomnames(AtomAttr):
@@ -426,6 +450,10 @@ class Atomnames(AtomAttr):
     per_object = 'atom'
     transplants = defaultdict(list)
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.array(['' for _ in range(na)], dtype=object)
+
     def getattr__(atomgroup, name):
         try:
             return atomgroup._get_named_atom(name)
@@ -585,6 +613,10 @@ class Atomtypes(AtomAttr):
     singular = 'type'
     per_object = 'atom'
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.array(['' for _ in range(na)], dtype=object)
+
 
 # TODO: update docs to property doc
 class Elements(AtomAttr):
@@ -592,6 +624,10 @@ class Elements(AtomAttr):
     attrname = 'elements'
     singular = 'element'
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.array(['' for _ in range(na)], dtype=object)
+
 
 # TODO: update docs to property doc
 class Radii(AtomAttr):
@@ -600,6 +636,10 @@ class Radii(AtomAttr):
     singular = 'radius'
     per_object = 'atom'
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.zeros(na)
+
 
 class ChainIDs(AtomAttr):
     """ChainID per atom
@@ -612,6 +652,10 @@ class ChainIDs(AtomAttr):
     singular = 'chainID'
     per_object = 'atom'
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.array(['' for _ in range(na)], dtype=object)
+
 
 class Tempfactors(AtomAttr):
     """Tempfactor for atoms"""
@@ -619,6 +663,10 @@ class Tempfactors(AtomAttr):
     singular = 'tempfactor'
     per_object = 'atom'
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.zeros(na)
+
 
 class Masses(AtomAttr):
     attrname = 'masses'
@@ -642,6 +690,10 @@ def __init__(self, values, guessed=False):
         self.values = np.asarray(values, dtype=np.float64)
         self._guessed = guessed
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.zeros(na)
+
     def get_residues(self, rg):
         resatoms = self.top.tt.residues2atoms_2d(rg.ix)
 
@@ -969,6 +1021,10 @@ class Charges(AtomAttr):
                       Atom, Residue, Segment]
     transplants = defaultdict(list)
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.zeros(na)
+
     def get_residues(self, rg):
         resatoms = self.top.tt.residues2atoms_2d(rg.ix)
 
@@ -1010,6 +1066,10 @@ class Bfactors(AtomAttr):
     singular = 'bfactor'
     per_object = 'atom'
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.zeros(na)
+
 
 # TODO: update docs to property doc
 class Occupancies(AtomAttr):
@@ -1017,6 +1077,10 @@ class Occupancies(AtomAttr):
     singular = 'occupancy'
     per_object = 'atom'
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.zeros(na)
+
 
 # TODO: update docs to property doc
 class AltLocs(AtomAttr):
@@ -1025,6 +1089,10 @@ class AltLocs(AtomAttr):
     singular = 'altLoc'
     per_object = 'atom'
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.array(['' for _ in range(na)], dtype=object)
+
 
 class ResidueAttr(TopologyAttr):
     attrname = 'residueattrs'
@@ -1066,6 +1134,10 @@ class Resids(ResidueAttr):
     singular = 'resid'
     target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Atom, Residue]
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.arange(nr) + 1
+
 
 # TODO: update docs to property doc
 class Resnames(ResidueAttr):
@@ -1074,6 +1146,10 @@ class Resnames(ResidueAttr):
     target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Atom, Residue]
     transplants = defaultdict(list)
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.array(['' for _ in range(nr)], dtype=object)
+
     def getattr__(residuegroup, resname):
         try:
             return residuegroup._get_named_residue(resname)
@@ -1231,12 +1307,20 @@ class Resnums(ResidueAttr):
     singular = 'resnum'
     target_classes = [AtomGroup, ResidueGroup, SegmentGroup, Atom, Residue]
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.arange(nr) + 1
+
 
 class ICodes(ResidueAttr):
     """Insertion code for Atoms"""
     attrname = 'icodes'
     singular = 'icode'
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.array(['' for _ in range(nr)], dtype=object)
+
 
 class Moltypes(ResidueAttr):
     """Name of the molecule type
@@ -1289,6 +1373,10 @@ class Segids(SegmentAttr):
                       Atom, Residue, Segment]
     transplants = defaultdict(list)
 
+    @staticmethod
+    def _gen_initial_values(na, nr, ns):
+        return np.array(['' for _ in range(ns)], dtype=object)
+
     def getattr__(segmentgroup, segid):
         try:
             return segmentgroup._get_named_segment(segid)
diff --git a/package/MDAnalysis/core/universe.py b/package/MDAnalysis/core/universe.py
index 1af0322b90f..6972cc7b722 100644
--- a/package/MDAnalysis/core/universe.py
+++ b/package/MDAnalysis/core/universe.py
@@ -93,7 +93,7 @@
 import MDAnalysis
 import sys
 
-from .. import _ANCHOR_UNIVERSES
+from .. import _ANCHOR_UNIVERSES, _TOPOLOGY_ATTRS
 from ..exceptions import NoDataError
 from ..lib import util
 from ..lib.log import ProgressMeter, _set_verbose
@@ -672,8 +672,37 @@ def trajectory(self, value):
         del self._trajectory  # guarantees that files are closed (?)
         self._trajectory = value
 
-    def add_TopologyAttr(self, topologyattr):
-        """Add a new topology attribute."""
+    def add_TopologyAttr(self, topologyattr, values=None):
+        """Add a new topology attribute to the Universe
+
+        Adding a TopologyAttribute to the Universe makes it available to
+        all AtomGroups etc throughout the Universe.
+
+        Parameters
+        ----------
+        topologyattr : TopologyAttr or string
+          Either a MDAnalysis TopologyAttr object or the name of a possible
+          topology attribute.
+        values : np.ndarray, optional
+          If initiating an attribute from a string, the initial values to
+          use.  If not supplied, the new TopologyAttribute will have empty
+          or zero values.
+        """
+        if isinstance(topologyattr, six.string_types):
+            try:
+                tcls = _TOPOLOGY_ATTRS[topologyattr]
+            except KeyError:
+                raise ValueError(
+                    "Unrecognised topology attribute name: '{}'."
+                    "  Possible values: '{}'".format(
+                        topologyattr, ', '.join(sorted(_TOPOLOGY_ATTRS.keys())))
+                )
+            else:
+                topologyattr = tcls.from_blank(
+                    n_atoms=self._topology.n_atoms,
+                    n_residues=self._topology.n_residues,
+                    n_segments=self._topology.n_segments,
+                    values=values)
         self._topology.add_TopologyAttr(topologyattr)
         self._process_attr(topologyattr)
 

From 000c5f2cbc170a91d78ca6ab35a666052bce711e Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Thu, 23 Nov 2017 00:30:49 +0000
Subject: [PATCH 10/23] Removed TestCase usage from test_universe

---
 .../MDAnalysisTests/core/test_universe.py     | 21 ++++++++-----------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/testsuite/MDAnalysisTests/core/test_universe.py b/testsuite/MDAnalysisTests/core/test_universe.py
index a4d75c9e829..22eeb32215c 100644
--- a/testsuite/MDAnalysisTests/core/test_universe.py
+++ b/testsuite/MDAnalysisTests/core/test_universe.py
@@ -25,7 +25,6 @@
 from six.moves import cPickle
 
 import os
-from unittest import TestCase
 
 try:
     from cStringIO import StringIO
@@ -285,7 +284,7 @@ def test_chainid_quick_select():
     assert len(u.D.atoms) == 7
 
 
-class TestGuessBonds(TestCase):
+class TestGuessBonds(object):
     """Test the AtomGroup methed guess_bonds
 
     This needs to be done both from Universe creation (via kwarg) and AtomGroup
@@ -295,11 +294,9 @@ class TestGuessBonds(TestCase):
      - fail properly if not
      - work again if vdwradii are passed.
     """
-    def setUp(self):
-        self.vdw = {'A':1.05, 'B':0.4}
-
-    def tearDown(self):
-        del self.vdw
+    @pytest.fixture()
+    def vdw(self):
+        return {'A': 1.05, 'B': 0.4}
 
     def _check_universe(self, u):
         """Verify that the Universe is created correctly"""
@@ -325,13 +322,13 @@ def test_universe_guess_bonds_no_vdwradii(self):
         with pytest.raises(ValueError):
             mda.Universe(two_water_gro_nonames, guess_bonds = True)
 
-    def test_universe_guess_bonds_with_vdwradii(self):
+    def test_universe_guess_bonds_with_vdwradii(self, vdw):
         """Unknown atom types, but with vdw radii here to save the day"""
         u = mda.Universe(two_water_gro_nonames, guess_bonds=True,
-                                vdwradii=self.vdw)
+                                vdwradii=vdw)
         self._check_universe(u)
         assert u.kwargs['guess_bonds']
-        assert_equal(self.vdw, u.kwargs['vdwradii'])
+        assert_equal(vdw, u.kwargs['vdwradii'])
 
     def test_universe_guess_bonds_off(self):
         u = mda.Universe(two_water_gro_nonames, guess_bonds=False)
@@ -372,11 +369,11 @@ def test_atomgroup_guess_bonds_no_vdwradii(self):
         with pytest.raises(ValueError):
             ag.guess_bonds()
 
-    def test_atomgroup_guess_bonds_with_vdwradii(self):
+    def test_atomgroup_guess_bonds_with_vdwradii(self, vdw):
         u = mda.Universe(two_water_gro_nonames)
 
         ag = u.atoms[:3]
-        ag.guess_bonds(vdwradii=self.vdw)
+        ag.guess_bonds(vdwradii=vdw)
         self._check_atomgroup(ag, u)
 
 

From 7c50845225710617cb3390c3e05674ea3241b297 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Thu, 23 Nov 2017 01:31:19 +0000
Subject: [PATCH 11/23] added tests for universe.add_TopologyAttr

---
 .../MDAnalysisTests/core/test_universe.py     | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/testsuite/MDAnalysisTests/core/test_universe.py b/testsuite/MDAnalysisTests/core/test_universe.py
index 22eeb32215c..3fd053e5ac0 100644
--- a/testsuite/MDAnalysisTests/core/test_universe.py
+++ b/testsuite/MDAnalysisTests/core/test_universe.py
@@ -525,3 +525,46 @@ def test_custom_both(self):
         u = mda.Universe(TRZ_psf, TRZ, format=MDAnalysis.coordinates.TRZ.TRZReader,
                          topology_format=MDAnalysis.topology.PSFParser.PSFParser)
         assert_equal(len(u.atoms), 8184)
+
+
+class TestAddTopologyAttr(object):
+    @pytest.fixture()
+    def universe(self):
+        return make_Universe()
+
+    def test_add_TA_fail(self, universe):
+        with pytest.raises(ValueError):
+            universe.add_TopologyAttr('silly')
+
+    def test_nodefault_fail(self, universe):
+        with pytest.raises(NotImplementedError):
+            universe.add_TopologyAttr('bonds')
+
+    @pytest.mark.parametrize(
+        'toadd,attrname,default', (
+            ['charge', 'charges', 0.0], ['charges', 'charges', 0.0],
+            ['name', 'names', ''], ['names', 'names', ''],
+            ['type', 'types', ''], ['types', 'types', ''],
+            ['element', 'elements', ''], ['elements', 'elements', ''],
+            ['radius', 'radii', 0.0], ['radii', 'radii', 0.0],
+            ['chainID', 'chainIDs', ''], ['chainIDs', 'chainIDs', ''],
+            ['tempfactor', 'tempfactors', 0.0],
+            ['tempfactors', 'tempfactors', 0.0],
+            ['mass', 'masses', 0.0], ['masses', 'masses', 0.0],
+            ['charge', 'charges', 0.0], ['charges', 'charges', 0.0],
+            ['bfactor', 'bfactors', 0.0], ['bfactors', 'bfactors', 0.0],
+            ['occupancy', 'occupancies', 0.0],
+            ['occupancies', 'occupancies', 0.0],
+            ['altLoc', 'altLocs', ''], ['altLocs', 'altLocs', ''],
+            ['resid', 'resids', 1], ['resids', 'resids', 1],
+            ['resname', 'resnames', ''], ['resnames', 'resnames', ''],
+            ['resnum', 'resnums', 1], ['resnums', 'resnums', 1],
+            ['icode', 'icodes', ''], ['icodes', 'icodes', ''],
+            ['segid', 'segids', ''], ['segids', 'segids', ''],
+        )
+    )
+    def test_add_charges(self, universe, toadd, attrname, default):
+        universe.add_TopologyAttr(toadd)
+
+        assert hasattr(universe.atoms, attrname)
+        assert getattr(universe.atoms, attrname)[0] == default

From db9a9e1179341bb735e1cabef820dd7bfafd6c0c Mon Sep 17 00:00:00 2001
From: Max Linke <max_linke@gmx.de>
Date: Sun, 3 Dec 2017 14:22:32 +0100
Subject: [PATCH 12/23] fix doc build error

Need to list arguments separately.
---
 package/MDAnalysis/core/topologyattrs.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/package/MDAnalysis/core/topologyattrs.py b/package/MDAnalysis/core/topologyattrs.py
index fb7c8704810..52e395c6615 100644
--- a/package/MDAnalysis/core/topologyattrs.py
+++ b/package/MDAnalysis/core/topologyattrs.py
@@ -226,8 +226,12 @@ def from_blank(cls, n_atoms=None, n_residues=None, n_segments=None,
 
         Parameters
         ----------
-        n_atoms, n_residues, n_segments : int, optional
-          Size of the TopologyAttribute
+        n_atoms : int, optional
+          Size of the TopologyAttribute atoms
+        n_residues: int, optional
+          Size of the TopologyAttribute residues
+        n_segments : int, optional
+          Size of the TopologyAttribute segments
         values : optional
           Initial values for the TopologyAttribute
         """

From fa837299508aedb74203053ade9e5d38677885be Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Sun, 3 Dec 2017 13:55:19 +0000
Subject: [PATCH 13/23] removed segments from WHITELIST

---
 package/MDAnalysis/core/groups.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py
index 68bf35216e2..f2745f51925 100644
--- a/package/MDAnalysis/core/groups.py
+++ b/package/MDAnalysis/core/groups.py
@@ -201,7 +201,7 @@ def _subclass(cls, singular):
         if singular:
             newcls._SETATTR_WHITELIST = {
                 'position', 'velocity', 'force', 'dimensions',
-                'atoms', 'residue', 'residues', 'segment', 'segments',
+                'atoms', 'residue', 'residues', 'segment',
             }
         else:
             newcls._SETATTR_WHITELIST = {

From b56254c70b1ee6c2f8012f3ea57de30564fe68c6 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Wed, 6 Dec 2017 10:35:30 +0000
Subject: [PATCH 14/23] Added example to add_TopologyAttr

Expandded missing TopologyAttr docstring
---
 package/MDAnalysis/core/universe.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/package/MDAnalysis/core/universe.py b/package/MDAnalysis/core/universe.py
index 6972cc7b722..aa8293286c7 100644
--- a/package/MDAnalysis/core/universe.py
+++ b/package/MDAnalysis/core/universe.py
@@ -687,6 +687,15 @@ def add_TopologyAttr(self, topologyattr, values=None):
           If initiating an attribute from a string, the initial values to
           use.  If not supplied, the new TopologyAttribute will have empty
           or zero values.
+
+        Example
+        -------
+        For example to add bfactors to a Universe:
+
+        >>> u.add_TopologyAttr('bfactors')
+        >>> u.atoms.bfactors
+        array([ 0.,  0.,  0., ...,  0.,  0.,  0.])
+
         """
         if isinstance(topologyattr, six.string_types):
             try:
@@ -694,7 +703,9 @@ def add_TopologyAttr(self, topologyattr, values=None):
             except KeyError:
                 raise ValueError(
                     "Unrecognised topology attribute name: '{}'."
-                    "  Possible values: '{}'".format(
+                    "  Possible values: '{}'\n"
+                    "To raise an issue go to: http://issues.mdanalysis.org"
+                    "".format(
                         topologyattr, ', '.join(sorted(_TOPOLOGY_ATTRS.keys())))
                 )
             else:

From e84c3fd6a1b84c43fc1c5e30da93f5f8d84b83b6 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Wed, 6 Dec 2017 10:35:56 +0000
Subject: [PATCH 15/23] Added explanation of _gen_initial_values

---
 package/MDAnalysis/core/topologyattrs.py | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/package/MDAnalysis/core/topologyattrs.py b/package/MDAnalysis/core/topologyattrs.py
index 52e395c6615..57dd1daeafd 100644
--- a/package/MDAnalysis/core/topologyattrs.py
+++ b/package/MDAnalysis/core/topologyattrs.py
@@ -217,6 +217,12 @@ def __init__(self, values, guessed=False):
 
     @staticmethod
     def _gen_initial_values(n_atoms, n_residues, n_segments):
+        """Populate an initial empty data structure for this Attribute
+
+        The only provided parameters are the "shape" of the Universe
+
+        Eg for charges, provide np.zeros(n_atoms)
+        """
         raise NotImplementedError("No default values")
 
     @classmethod
@@ -442,7 +448,7 @@ class Atomids(AtomAttr):
 
     @staticmethod
     def _gen_initial_values(na, nr, ns):
-        return np.arange(na) + 1
+        return np.arange(1, na + 1)
 
 
 # TODO: update docs to property doc
@@ -1140,7 +1146,7 @@ class Resids(ResidueAttr):
 
     @staticmethod
     def _gen_initial_values(na, nr, ns):
-        return np.arange(nr) + 1
+        return np.arange(1, nr + 1)
 
 
 # TODO: update docs to property doc
@@ -1313,7 +1319,7 @@ class Resnums(ResidueAttr):
 
     @staticmethod
     def _gen_initial_values(na, nr, ns):
-        return np.arange(nr) + 1
+        return np.arange(1, nr + 1)
 
 
 class ICodes(ResidueAttr):

From 68c2da4cc2598ee2d44f36b1c31c7a05a4e42ef5 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Wed, 6 Dec 2017 10:36:37 +0000
Subject: [PATCH 16/23] Renamed _TopologyAttrContainer._singular to _is_group

---
 package/MDAnalysis/core/groups.py | 44 +++++++++++++++----------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py
index f2745f51925..b2b8b32919a 100644
--- a/package/MDAnalysis/core/groups.py
+++ b/package/MDAnalysis/core/groups.py
@@ -153,13 +153,14 @@ def make_classes():
 
     # The 'GBase' middle man is needed so that a single topologyattr
     #  patching applies automatically to all groups.
-    GBase = bases[GroupBase] = _TopologyAttrContainer._subclass(singular=False)
+    GBase = bases[GroupBase] = _TopologyAttrContainer._subclass(is_group=True)
     for cls in groups:
-        bases[cls] = GBase._subclass(singular=False)
+        bases[cls] = GBase._subclass(is_group=True)
     # CBase for patching all components
-    CBase = bases[ComponentBase] = _TopologyAttrContainer._subclass(singular=True)
+    CBase = bases[ComponentBase] = _TopologyAttrContainer._subclass(
+        is_group=False)
     for cls in components:
-        bases[cls] = CBase._subclass(singular=True)
+        bases[cls] = CBase._subclass(is_group=False)
 
     # Initializes the class cache.
     for cls in groups + components:
@@ -183,30 +184,30 @@ class _TopologyAttrContainer(object):
       :class:`Universe`.
     """
     @classmethod
-    def _subclass(cls, singular):
+    def _subclass(cls, is_group):
         """Factory method returning :class:`_TopologyAttrContainer` subclasses.
 
         Parameters
         ----------
-        singular : bool
-            The :attr:`_singular` of the returned class will be set to
-            *singular*. It controls the type of :class:`TopologyAttr` addition.
+        is_group : bool
+            The :attr:`_is_group` of the returned class will be set to
+            *is_group*. It controls the type of :class:`TopologyAttr` addition.
 
         Returns
         -------
         type
             A subclass of :class:`_TopologyAttrContainer`, with the same name.
         """
-        newcls = type(cls.__name__, (cls,), {'_singular': bool(singular)})
-        if singular:
+        newcls = type(cls.__name__, (cls,), {'_is_group': bool(is_group)})
+        if is_group:
             newcls._SETATTR_WHITELIST = {
-                'position', 'velocity', 'force', 'dimensions',
-                'atoms', 'residue', 'residues', 'segment',
+                'positions', 'velocities', 'forces', 'dimensions',
+                'atoms', 'residue', 'residues', 'segment', 'segments',
             }
         else:
             newcls._SETATTR_WHITELIST = {
-                'positions', 'velocities', 'forces', 'dimensions',
-                'atoms', 'residue', 'residues', 'segment', 'segments',
+                'position', 'velocity', 'force', 'dimensions',
+                'atoms', 'residue', 'residues', 'segment',
             }
 
         return newcls
@@ -254,17 +255,14 @@ def getter(self):
         def setter(self, values):
             return attr.__setitem__(self, values)
 
-        if cls._singular:
-            setattr(cls, attr.singular,
-                    property(getter, setter, None, attr.singledoc))
-        else:
+        if cls._is_group:
             setattr(cls, attr.attrname,
                     property(getter, setter, None, attr.groupdoc))
-
-        if cls._singular:
-            cls._SETATTR_WHITELIST.add(attr.singular)
-        else:
             cls._SETATTR_WHITELIST.add(attr.attrname)
+        else:
+            setattr(cls, attr.singular,
+                    property(getter, setter, None, attr.singledoc))
+            cls._SETATTR_WHITELIST.add(attr.singular)
 
     def __setattr__(self, attr, value):
         # `ag.this = 42` calls setattr(ag, 'this', 42)
@@ -273,7 +271,7 @@ def __setattr__(self, attr, value):
                 hasattr(self, attr)):  # preexisting (eg properties) allowed
             raise AttributeError(
                 "Cannot set arbitrary attributes to a {}".format(
-                    'Component' if self._singular else 'Group'))
+                    'Group' if self._is_group else 'Component'))
         # if it is, we allow the setattr to proceed by deferring to the super
         # behaviour (ie do it)
         super(_TopologyAttrContainer, self).__setattr__(attr, value)

From d37b6699aa0e94abe3ebcbd21fad29428a669390 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Wed, 6 Dec 2017 11:31:09 +0000
Subject: [PATCH 17/23] clarified UAG upadte procedure

---
 package/MDAnalysis/core/groups.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py
index b2b8b32919a..8f0ecfae42d 100644
--- a/package/MDAnalysis/core/groups.py
+++ b/package/MDAnalysis/core/groups.py
@@ -2877,7 +2877,8 @@ def _ensure_updated(self):
 
     def __getattribute__(self, name):
         # ALL attribute access goes through here
-        # If the requested attribute isn't in the shortcut list, update ourselves
+        # If the requested attribute is public (not starting with '_') and
+        # isn't in the shortcut list, update ourselves
         if not (name.startswith('_') or name in _UAG_SHORTCUT_ATTRS):
             self._ensure_updated()
         # Going via object.__getattribute__ then bypasses this check stage

From 4dcfb7d4c5c45e64689ed9e5aab5eec51a30901a Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Wed, 6 Dec 2017 11:37:47 +0000
Subject: [PATCH 18/23] Changelog for attribute setting changes

---
 package/CHANGELOG | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/package/CHANGELOG b/package/CHANGELOG
index eb14bbd77f6..192c08e19b8 100644
--- a/package/CHANGELOG
+++ b/package/CHANGELOG
@@ -30,6 +30,8 @@ Enhancements
   * added support for Tinker TXYZ and ARC files
   * libmdaxdr and libdcd classes can now be pickled (PR #1680)
   * speed up atomgroup.rotate when point = (0, 0, 0) (Issue #1707)
+  * Universe.add_TopologyAttr now accepts strings to add a given attribute
+    to the Universe (Issue #1092, PR #1186)
 
 Deprecations
 
@@ -81,6 +83,8 @@ Changes
     tuple (filename_or_array, trajectory_type) (Issue #1613)
   * docs: URLs to (www|docs).mdanalysis.org now link to SSL-encrypted site
     (see issue MDAnalysis/MDAnalysis.github.io#61)
+  * attributes can not be assigned to AtomGroups (and similar objects) unless
+    they are part of the Universe topology (Issue #1092 PR #1186)
 
 
 06/29/17 richardjgowers, rathann, jbarnoud, orbeckst, utkbansal
@@ -965,7 +969,7 @@ Fixes
 
 04/01/14  orbeckst, jandom, zhuyi.xue, xdeupi, tyler.je.reddy,
           manuel.nuno.melo, danny.parton, sebastien.buchoux, denniej0,
-	  rmcgibbo, richardjgowers, lennardvanderfeltz, bernardin.alejandro
+    rmcgibbo, richardjgowers, lennardvanderfeltz, bernardin.alejandro
           matthieu.chavent
 
   * 0.8.1
@@ -1480,7 +1484,7 @@ Fixes
     alternative atoms and insertion codes that are not needed for
     handling MD simulation data).
       - The default behaviour of MDAnalysis  can be set through the flag
-	MDAnalysis.core.flag['permissive_pdb_reader']. The default is True.
+  MDAnalysis.core.flag['permissive_pdb_reader']. The default is True.
       - One can always manually select the PDB reader by providing the
         permissive keyword to Universe; e.g. Universe(...,permissive=False)
         will read the input file with the Bio.PDB reader. This might be

From b9db71d547051e6ad32ef422cbada0ad27b3572c Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Wed, 6 Dec 2017 11:41:51 +0000
Subject: [PATCH 19/23] Small fix to Bond TopologyAttrs

removed tolist() call
use .universe not ._u
---
 package/MDAnalysis/core/topologyattrs.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/package/MDAnalysis/core/topologyattrs.py b/package/MDAnalysis/core/topologyattrs.py
index 57dd1daeafd..eefc1e5feea 100644
--- a/package/MDAnalysis/core/topologyattrs.py
+++ b/package/MDAnalysis/core/topologyattrs.py
@@ -1491,11 +1491,11 @@ def get_atoms(self, ag):
             unique_bonds = self._bondDict[ag.ix]
         bond_idx, types, guessed, order = np.hsplit(
             np.array(sorted(unique_bonds)), 4)
-        bond_idx = np.array(bond_idx.ravel().tolist(), dtype=np.int32)
+        bond_idx = np.array(bond_idx.ravel(), dtype=np.int32)
         types = types.ravel()
         guessed = guessed.ravel()
         order = order.ravel()
-        return TopologyGroup(bond_idx, ag._u,
+        return TopologyGroup(bond_idx, ag.universe,
                              self.singular[:-1],
                              types,
                              guessed,
@@ -1542,7 +1542,7 @@ class Bonds(_Connection):
     def bonded_atoms(self):
         """An AtomGroup of all atoms bonded to this Atom"""
         idx = [b.partner(self).index for b in self.bonds]
-        return self._u.atoms[idx]
+        return self.universe.atoms[idx]
 
     transplants[Atom].append(
         ('bonded_atoms', property(bonded_atoms, None, None,

From 3c4d607d6c0fb1bb7ec6141afff50b7f353d8557 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Wed, 6 Dec 2017 11:47:12 +0000
Subject: [PATCH 20/23] Further clarified is_group in
 TopologyAttrContainer.subclass

---
 package/MDAnalysis/core/groups.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py
index 8f0ecfae42d..ab8ba30a9ac 100644
--- a/package/MDAnalysis/core/groups.py
+++ b/package/MDAnalysis/core/groups.py
@@ -191,7 +191,10 @@ def _subclass(cls, is_group):
         ----------
         is_group : bool
             The :attr:`_is_group` of the returned class will be set to
-            *is_group*. It controls the type of :class:`TopologyAttr` addition.
+            *is_group*. This is used to distinguish between Groups (AtomGroup
+            etc.) and Components (Atom etc.) in internal methods when
+            considering actions such as addition between objects, adding
+            TopologyAttributes to them.
 
         Returns
         -------

From 958e971d1e2449d3865af4789c2b454dc3594368 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Wed, 6 Dec 2017 17:04:16 +0000
Subject: [PATCH 21/23] reverted change to Bond TopologyAttr

---
 package/MDAnalysis/core/topologyattrs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/MDAnalysis/core/topologyattrs.py b/package/MDAnalysis/core/topologyattrs.py
index eefc1e5feea..f9e284fcb23 100644
--- a/package/MDAnalysis/core/topologyattrs.py
+++ b/package/MDAnalysis/core/topologyattrs.py
@@ -1491,7 +1491,7 @@ def get_atoms(self, ag):
             unique_bonds = self._bondDict[ag.ix]
         bond_idx, types, guessed, order = np.hsplit(
             np.array(sorted(unique_bonds)), 4)
-        bond_idx = np.array(bond_idx.ravel(), dtype=np.int32)
+        bond_idx = np.array(bond_idx.ravel().tolist(), dtype=np.int32)
         types = types.ravel()
         guessed = guessed.ravel()
         order = order.ravel()

From e7104a5a8aa4a683931a6f89303bd6d6d9bf5df8 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Wed, 6 Dec 2017 18:14:33 +0000
Subject: [PATCH 22/23] Fixed up CHANGELOG

---
 package/CHANGELOG | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/package/CHANGELOG b/package/CHANGELOG
index 192c08e19b8..04615d57bcc 100644
--- a/package/CHANGELOG
+++ b/package/CHANGELOG
@@ -969,7 +969,7 @@ Fixes
 
 04/01/14  orbeckst, jandom, zhuyi.xue, xdeupi, tyler.je.reddy,
           manuel.nuno.melo, danny.parton, sebastien.buchoux, denniej0,
-    rmcgibbo, richardjgowers, lennardvanderfeltz, bernardin.alejandro
+          rmcgibbo, richardjgowers, lennardvanderfeltz, bernardin.alejandro
           matthieu.chavent
 
   * 0.8.1
@@ -1484,7 +1484,7 @@ Fixes
     alternative atoms and insertion codes that are not needed for
     handling MD simulation data).
       - The default behaviour of MDAnalysis  can be set through the flag
-  MDAnalysis.core.flag['permissive_pdb_reader']. The default is True.
+        MDAnalysis.core.flag['permissive_pdb_reader']. The default is True.
       - One can always manually select the PDB reader by providing the
         permissive keyword to Universe; e.g. Universe(...,permissive=False)
         will read the input file with the Bio.PDB reader. This might be

From de695f4048fae9e63b0e675a97db8255648abd14 Mon Sep 17 00:00:00 2001
From: Richard Gowers <richardjgowers@gmail.com>
Date: Wed, 6 Dec 2017 18:16:24 +0000
Subject: [PATCH 23/23] Updated Universe.add_TopologyAttr docstring

---
 package/MDAnalysis/core/universe.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/MDAnalysis/core/universe.py b/package/MDAnalysis/core/universe.py
index aa8293286c7..fb8c006e468 100644
--- a/package/MDAnalysis/core/universe.py
+++ b/package/MDAnalysis/core/universe.py
@@ -696,6 +696,10 @@ def add_TopologyAttr(self, topologyattr, values=None):
         >>> u.atoms.bfactors
         array([ 0.,  0.,  0., ...,  0.,  0.,  0.])
 
+        .. versionchanged:: 0.17.0
+           Can now also add TopologyAttrs with a string of the name of the
+           attribute to add (eg 'charges'), can also supply initial values
+           using values keyword.
         """
         if isinstance(topologyattr, six.string_types):
             try: