From 2737e38070172ac1294d3baa62374e66057d4383 Mon Sep 17 00:00:00 2001 From: David Dotson Date: Sat, 2 Apr 2016 14:49:09 -0700 Subject: [PATCH 1/7] Added automatic grabbing of kwargs from Universe. The MDAnalysis.Universe does not currently hold onto its kwargs, so this is dependent on a small upstream change. --- src/mdsynthesis/limbs.py | 10 +++++++++- src/mdsynthesis/treants.py | 22 ++++++---------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/mdsynthesis/limbs.py b/src/mdsynthesis/limbs.py index bec0677..c30c094 100644 --- a/src/mdsynthesis/limbs.py +++ b/src/mdsynthesis/limbs.py @@ -29,7 +29,7 @@ class UniverseDefinition(Limb): this universe directly available via ``Sim.atomselections``. """ - _name = 'udef' + _name = '_udef' _filepaths = ['abs', 'rel'] def __init__(self, treant): @@ -295,6 +295,14 @@ def kwargs(self, kwargs): if not isinstance(kwargs, dict): raise TypeError("Must be a dictionary") + # check that values are serializable + for key, value in kwargs.items(): + if not (isinstance(value, (string_types, bool, int, float)) or + value is None): + raise ValueError("Cannot store keyword '{}' for Universe; " + "value must be a string, bool, int, float, " + "or None, not '{}'".format(key, type(value))) + with self._treant._write: simdict = self._treant._state['mdsynthesis']['udef'] simdict['kwargs'] = kwargs diff --git a/src/mdsynthesis/treants.py b/src/mdsynthesis/treants.py index bd40a3e..048ac6e 100644 --- a/src/mdsynthesis/treants.py +++ b/src/mdsynthesis/treants.py @@ -54,7 +54,7 @@ def __init__(self, sim, new=False, categories=None, tags=None): categories=categories, tags=tags) - self._udef = None + self._udef = limbs.UniverseDefinition(self) self._atomselections = None self._universe = None # universe 'dock' @@ -80,7 +80,7 @@ def universe(self): if self._universe: return self._universe else: - self.udef._activate() + self._udef._activate() return self._universe @universe.setter @@ -89,7 +89,7 @@ def universe(self, universe): raise TypeError("Cannot set to {}; must be Universe".format( type(universe))) - self.udef.topology = universe.filename + self._udef.topology = universe.filename try: traj = universe.trajectory.filename except AttributeError: @@ -98,23 +98,13 @@ def universe(self, universe): except AttributeError: traj = None - self.udef.trajectory = traj + self._udef.trajectory = traj + + self._udef.kwargs = universe._kwargs # finally, just use this instance self._universe = universe - @property - def udef(self): - """The universe definition for this Sim. - - """ - # attach universe if not attached, and only give results if a - # universe is present thereafter - if not self._udef: - self._udef = limbs.UniverseDefinition(self) - - return self._udef - @property def atomselections(self): """Stored atom selections for the universe. From 2d49edd6a6f1b9989931aaaa2353f10ee2361ff7 Mon Sep 17 00:00:00 2001 From: David Dotson Date: Sat, 2 Apr 2016 14:57:39 -0700 Subject: [PATCH 2/7] Wrapped autokwarg pulling in try-except so it works with older MDA --- src/mdsynthesis/treants.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/mdsynthesis/treants.py b/src/mdsynthesis/treants.py index 048ac6e..beb9c95 100644 --- a/src/mdsynthesis/treants.py +++ b/src/mdsynthesis/treants.py @@ -54,7 +54,7 @@ def __init__(self, sim, new=False, categories=None, tags=None): categories=categories, tags=tags) - self._udef = limbs.UniverseDefinition(self) + self._udef = None self._atomselections = None self._universe = None # universe 'dock' @@ -80,7 +80,7 @@ def universe(self): if self._universe: return self._universe else: - self._udef._activate() + self.udef._activate() return self._universe @universe.setter @@ -89,7 +89,7 @@ def universe(self, universe): raise TypeError("Cannot set to {}; must be Universe".format( type(universe))) - self._udef.topology = universe.filename + self.udef.topology = universe.filename try: traj = universe.trajectory.filename except AttributeError: @@ -98,13 +98,30 @@ def universe(self, universe): except AttributeError: traj = None - self._udef.trajectory = traj + self.udef.trajectory = traj - self._udef.kwargs = universe._kwargs + # try and store keyword arguments + try: + self.udef.kwargs = universe._kwargs + except AttributeError: + warnings.warn("Could not store universe keyword arguments; " + "manually set these with ``Sim.udef.kwargs``") # finally, just use this instance self._universe = universe + @property + def udef(self): + """The universe definition for this Sim. + + """ + # attach universe if not attached, and only give results if a + # universe is present thereafter + if not self._udef: + self._udef = limbs.UniverseDefinition(self) + + return self._udef + @property def atomselections(self): """Stored atom selections for the universe. From 5ec58e8238418c77d60a4af68e7d2ae7ddb5efba Mon Sep 17 00:00:00 2001 From: David Dotson Date: Sun, 3 Apr 2016 10:08:45 -0700 Subject: [PATCH 3/7] Should now be able to set universe to None This will remove the universe definition for the Sim --- src/mdsynthesis/limbs.py | 2 +- src/mdsynthesis/treants.py | 46 +++++++++++++++++++++----------------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/mdsynthesis/limbs.py b/src/mdsynthesis/limbs.py index c30c094..f7bc60d 100644 --- a/src/mdsynthesis/limbs.py +++ b/src/mdsynthesis/limbs.py @@ -29,7 +29,7 @@ class UniverseDefinition(Limb): this universe directly available via ``Sim.atomselections``. """ - _name = '_udef' + _name = 'udef' _filepaths = ['abs', 'rel'] def __init__(self, treant): diff --git a/src/mdsynthesis/treants.py b/src/mdsynthesis/treants.py index beb9c95..ddd20e6 100644 --- a/src/mdsynthesis/treants.py +++ b/src/mdsynthesis/treants.py @@ -69,10 +69,8 @@ def universe(self): for this universe directly available via ``Sim.selections``. Setting this to a :class:`MDAnalysis.Universe` will set that as the - universe definition for this Sim. It will not preserve any keyword - arguments used to initialize it, however; you will need to add - these to :attr:`universe_kwargs` as a dictionary if you want these - to apply next time the universe is loaded. + universe definition for this Sim. Setting to ``None`` will remove + the universe definition entirely. """ # TODO: include check for changes to universe definition, not just @@ -85,30 +83,36 @@ def universe(self): @universe.setter def universe(self, universe): - if not isinstance(universe, Universe): + if universe is None: + self.udef.topology = None + self.udef.trajectory = None + self.udef.kwargs = None + self._universe = None + + elif not isinstance(universe, Universe): raise TypeError("Cannot set to {}; must be Universe".format( type(universe))) - - self.udef.topology = universe.filename - try: - traj = universe.trajectory.filename - except AttributeError: + else: + self.udef.topology = universe.filename try: - traj = universe.trajectory.filenames + traj = universe.trajectory.filename except AttributeError: - traj = None + try: + traj = universe.trajectory.filenames + except AttributeError: + traj = None - self.udef.trajectory = traj + self.udef.trajectory = traj - # try and store keyword arguments - try: - self.udef.kwargs = universe._kwargs - except AttributeError: - warnings.warn("Could not store universe keyword arguments; " - "manually set these with ``Sim.udef.kwargs``") + # try and store keyword arguments + try: + self.udef.kwargs = universe._kwargs + except AttributeError: + warnings.warn("Could not store universe keyword arguments; " + "manually set these with ``Sim.udef.kwargs``") - # finally, just use this instance - self._universe = universe + # finally, just use this instance + self._universe = universe @property def udef(self): From 4dc9061bf5042f71f9244c11d9ba2e93ede4a929 Mon Sep 17 00:00:00 2001 From: David Dotson Date: Sun, 3 Apr 2016 11:16:40 -0700 Subject: [PATCH 4/7] Basic test added for setting a universe with kwargs. --- src/mdsynthesis/tests/test_treants.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/mdsynthesis/tests/test_treants.py b/src/mdsynthesis/tests/test_treants.py index dc1cef3..fe236bc 100644 --- a/src/mdsynthesis/tests/test_treants.py +++ b/src/mdsynthesis/tests/test_treants.py @@ -74,6 +74,21 @@ def test_set_universe_chainreader(self, treant): assert treant.udef.topology == GRO assert treant.udef.trajectory == [XTC, XTC] + def test_set_universe_with_kwargs(self, treant): + """Universe should preserve its kwargs, if possible. + + Test that setting a Universe for a Sim also gets its kwargs + preserved, that an exception is raised for unserializable kwargs, + and that a proper warning is geven when the Sim can't get them from + the Universe in the first place. + + """ + u = mda.Universe(PDB, XTC, something_fake=True) + + treant.universe = u + + assert treant.udef.kwargs['something_fake'] is True + def test_add_univese_typeerror(self, treant): """Test checking of what is passed to setter""" with pytest.raises(TypeError): From cd406417312a21032c604f38a29624ec12ecc053 Mon Sep 17 00:00:00 2001 From: David Dotson Date: Sun, 3 Apr 2016 14:45:40 -0700 Subject: [PATCH 5/7] Added tests for kwarg storage behavior. --- src/mdsynthesis/tests/test_treants.py | 15 ++++++++++++++- src/mdsynthesis/treants.py | 4 ++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/mdsynthesis/tests/test_treants.py b/src/mdsynthesis/tests/test_treants.py index fe236bc..6970c2a 100644 --- a/src/mdsynthesis/tests/test_treants.py +++ b/src/mdsynthesis/tests/test_treants.py @@ -83,12 +83,25 @@ def test_set_universe_with_kwargs(self, treant): the Universe in the first place. """ + # try out a basic, but nonexistent, kwarg u = mda.Universe(PDB, XTC, something_fake=True) treant.universe = u - assert treant.udef.kwargs['something_fake'] is True + # try a kwarg with a value that won't be serializable + u2 = mda.Universe(PDB, XTC, + something_else=mda.topology.GROParser.GROParser) + + with pytest.raises(ValueError): + treant.universe = u2 + + # check that we get a warning if a Universe didn't store its kwargs + u3 = mda.Universe(PDB, XTC, something_fake=True) + del u3._kwargs + with pytest.warns(UserWarning): + treant.universe = u3 + def test_add_univese_typeerror(self, treant): """Test checking of what is passed to setter""" with pytest.raises(TypeError): diff --git a/src/mdsynthesis/treants.py b/src/mdsynthesis/treants.py index 16c0856..202f8a8 100644 --- a/src/mdsynthesis/treants.py +++ b/src/mdsynthesis/treants.py @@ -108,8 +108,8 @@ def universe(self, universe): try: self.udef.kwargs = universe._kwargs except AttributeError: - warnings.warn("Could not store universe keyword arguments; " - "manually set these with ``Sim.udef.kwargs``") + warnings.warn("Universe did not keep keyword arguments; " + "cannot store keyword arguments for Universe.") # finally, just use this instance self._universe = universe From 2a5e6a63ec08174e0608cf8bd4c4a2cc83108d07 Mon Sep 17 00:00:00 2001 From: David Dotson Date: Mon, 4 Apr 2016 16:04:06 -0700 Subject: [PATCH 6/7] Refactor in light of discussion in #48. We will encourage setting a Sim universe directly with an existing Universe, with the hope that we can eventually hide UniverseDefinition. At the moment, though, we leave it exposed for any manual interventions that can't be handled from the Universe itself. Also, AtomSelection getitem no longer returns an AtomGroup, but instead gives back the definition of the selection (that which was stored). It is possible to store only strings or numpy arrays of atom indices, or tuples/lists of these in any combination. One cannot directly store AtomGroups, but can store atom indices with `AG.indices`. Getting back an AtomGroup from the stored definition requires using the `create` method. This makes it clear that a new AtomGroup is being generated, and that there is a cost to this. --- src/mdsynthesis/limbs.py | 184 ++++++++++++----------- src/mdsynthesis/tests/test_filesystem.py | 18 +-- src/mdsynthesis/tests/test_treants.py | 115 +++++++++----- src/mdsynthesis/treants.py | 37 +++-- 4 files changed, 205 insertions(+), 149 deletions(-) diff --git a/src/mdsynthesis/limbs.py b/src/mdsynthesis/limbs.py index f7bc60d..6ae6c3b 100644 --- a/src/mdsynthesis/limbs.py +++ b/src/mdsynthesis/limbs.py @@ -29,7 +29,7 @@ class UniverseDefinition(Limb): this universe directly available via ``Sim.atomselections``. """ - _name = 'udef' + _name = 'universedef' _filepaths = ['abs', 'rel'] def __init__(self, treant): @@ -114,7 +114,8 @@ def topology(self): """ with self._treant._read: - topstate = self._treant._state['mdsynthesis']['udef']['topology'] + mdsdict = self._treant._state['mdsynthesis'] + topstate = mdsdict['universedef']['topology'] if not topstate: return None @@ -140,10 +141,11 @@ def topology(self, path): def _set_topology(self, path): with self._treant._write: - topstate = self._treant._state['mdsynthesis']['udef']['topology'] + mdsdict = self._treant._state['mdsynthesis'] + topstate = mdsdict['universedef']['topology'] if path is None: - self._treant._state['mdsynthesis']['udef']['topology'] = dict() + mdsdict['universedef']['topology'] = dict() else: topstate['abspath'] = os.path.abspath(path) topstate['relpath'] = os.path.relpath(path, @@ -161,13 +163,14 @@ def trajectory(self): """ with self._treant._read: - traj = self._treant._state['mdsynthesis']['udef']['trajectory'] + mdsdict = self._treant._state['mdsynthesis'] + traj = mdsdict['universedef']['trajectory'] if not traj: return None elif len(traj) == 1: return traj[0][0] else: - return [t[0] for t in traj] + return tuple([t[0] for t in traj]) @trajectory.setter def trajectory(self, path): @@ -191,9 +194,9 @@ def trajectory(self, path): def _set_trajectory(self, trajs): with self._treant._write: - self._treant._state['mdsynthesis']['udef']['trajectory'] = [] mdsdict = self._treant._state['mdsynthesis'] - trajstate = mdsdict['udef']['trajectory'] + mdsdict['universedef']['trajectory'] = [] + trajstate = mdsdict['universedef']['trajectory'] for traj in trajs: trajstate.append( @@ -205,7 +208,7 @@ def _apply_resnums(self): """ with self._treant._read: - simdict = self._treant._state['mdsynthesis']['udef'] + simdict = self._treant._state['mdsynthesis']['universedef'] try: resnums = simdict['resnums'] except KeyError: @@ -232,7 +235,7 @@ def _set_resnums(self, resnums): resnum definition """ with self._treant._write: - simdict = self._treant._state['mdsynthesis']['udef'] + simdict = self._treant._state['mdsynthesis']['universedef'] if resnums is None: simdict['resnums'] = None else: @@ -263,11 +266,12 @@ def _define(self, pathtype='abs'): """ with self._treant._read: - top = self._treant._state['mdsynthesis']['udef']['topology'] + mdsdict = self._treant._state['mdsynthesis'] + top = mdsdict['universedef']['topology'] outtop = {'abs': top['abspath'], 'rel': top['relpath']} - trajs = self._treant._state['mdsynthesis']['udef']['trajectory'] + trajs = mdsdict['universedef']['trajectory'] outtraj = {key: [] for key in self._filepaths} for traj in trajs: @@ -288,23 +292,26 @@ def kwargs(self): """ with self._treant._read: mdsdict = self._treant._state['mdsynthesis'] - return mdsdict['udef']['kwargs'] + return mdsdict['universedef']['kwargs'] @kwargs.setter def kwargs(self, kwargs): - if not isinstance(kwargs, dict): - raise TypeError("Must be a dictionary") - - # check that values are serializable - for key, value in kwargs.items(): - if not (isinstance(value, (string_types, bool, int, float)) or - value is None): - raise ValueError("Cannot store keyword '{}' for Universe; " - "value must be a string, bool, int, float, " - "or None, not '{}'".format(key, type(value))) + if kwargs is None: + pass + elif isinstance(kwargs, dict): + # check that values are serializable + for key, value in kwargs.items(): + if not (isinstance(value, (string_types, bool, int, float)) or + value is None): + raise ValueError("Cannot store keyword '{}' for Universe; " + "value must be a string, bool, int, " + "float, or ``None``, " + "not '{}'".format(key, type(value))) + else: + raise TypeError("Must be a dictionary or ``None``") with self._treant._write: - simdict = self._treant._state['mdsynthesis']['udef'] + simdict = self._treant._state['mdsynthesis']['universedef'] simdict['kwargs'] = kwargs @@ -340,53 +347,29 @@ def __init__(self, treant): def __repr__(self): return "".format( - {x: self.define(x) for x in self.keys()}) - - def __str__(self): - selections = self.keys() - agg = "Selections" - majsep = "=" - minsep = "-" - subsep = "| " - seplength = len(agg) - - if not self._treant._uname: - out = "No universe attached; no Selections to show" - elif not selections: - out = "No selections for universe '{}'".format( - self._treant._uname) - else: - out = agg + '\n' - out = out + majsep * seplength + '\n' - for selection in selections: - out = out + "'{}'\n".format(selection) - for item in self.define(selection): - out = out + subsep + "'{}'\n".format(item) - out = out + minsep * seplength + '\n' - - return out + {x: self.get(x) for x in self.keys()}) def __getitem__(self, handle): - """Get selection as an AtomGroup for given handle and the universe. + """Get selection for given handle. Parameters ---------- handle : str - Name of selection to return as an AtomGroup. + Name of selection to return. Returns ------- - AtomGroup - The named selection as an AtomGroup of the universe. + selection : str, list, array_like + The named selection definition. """ - return self._asAtomGroup(handle) + return self.get(handle) def __setitem__(self, handle, selection): - """Selection for the given handle and the active universe. + """Selection for the given handle. """ - if isinstance(selection, (string_types, AtomGroup)): + if isinstance(selection, (string_types, np.ndarray)): selection = [selection] self.add(handle, *selection) @@ -406,32 +389,36 @@ def add(self, handle, *selection): data. It is useful to store AtomGroup selections for later use, since they can be complex and atom order may matter. - If a selection with the given *handle* already exists, it is replaced. + If a selection with the given `handle` already exists, it is replaced. Parameters ---------- handle : str Name to use for the selection. selection : str, AtomGroup - Selection string or AtomGroup; multiple selections may be given and - their order will be preserved, which is useful for e.g. structural - alignments. - """ - # Conversion function, leave strings alone, - # turn AtomGroups into their indices - def conv(x): - return x if isinstance(x, string_types) else x.indices - - selection = map(conv, selection) + Selection string or AtomGroup indices; multiple selections may be + given and their order will be preserved, which is useful for e.g. + structural alignments. - with self._treant._write: + """ + if len(selection) == 1: + sel = selection[0] + if isinstance(sel, np.ndarray): + outsel = sel.tolist() + elif isinstance(sel, string_types): + outsel = sel + else: outsel = list() for sel in selection: if isinstance(sel, np.ndarray): outsel.append(sel.tolist()) elif isinstance(sel, string_types): outsel.append(sel) + else: + raise ValueError("Selections must be strings, arrays of " + "atom indices, or tuples/lists of these.") + with self._treant._write: seldict = self._treant._state['mdsynthesis']['atomselections'] seldict[handle] = outsel @@ -462,8 +449,8 @@ def keys(self): seldict = self._treant._state['mdsynthesis']['atomselections'] return seldict.keys() - def _asAtomGroup(self, handle): - """Get AtomGroup from universe from the given named selection. + def create(self, handle): + """Generate AtomGroup from universe from the given named selection. If named selection doesn't exist, :exc:`KeyError` raised. @@ -476,29 +463,39 @@ def _asAtomGroup(self, handle): ------- AtomGroup The named selection as an AtomGroup of the universe. + """ - sel = self.define(handle) + sel = self.get(handle) # Selections might be either + # - a single string + # - a single list of indices # - a list of strings # - a list of indices - ag = None - for item in sel: - if isinstance(item, string_types): - if ag: - ag += self._treant.universe.select_atoms(item) - else: - ag = self._treant.universe.select_atoms(item) - else: - if ag: - ag += self._treant.universe.atoms[item] + if isinstance(sel, string_types): + # if we have a single string + ag = self._treant.universe.select_atoms(sel) + elif all([isinstance(i, int) for i in sel]): + # if we have a single array_like of indices + ag = self._treant.universe.atoms[sel] + else: + ag = None + for item in sel: + if isinstance(item, string_types): + if ag: + ag += self._treant.universe.select_atoms(item) + else: + ag = self._treant.universe.select_atoms(item) else: - ag = self._treant.universe.atoms[item] + if ag: + ag += self._treant.universe.atoms[item] + else: + ag = self._treant.universe.atoms[item] return ag - def define(self, handle): + def get(self, handle): """Get selection definition for given handle. If named selection doesn't exist, :exc:`KeyError` raised. @@ -512,13 +509,30 @@ def define(self, handle): ------- definition : list list of strings defining the atom selection + """ with self._treant._read: seldict = self._treant._state['mdsynthesis']['atomselections'] try: - selstring = seldict[handle] + seldef = seldict[handle] except KeyError: raise KeyError("No such selection '{}'".format(handle)) - return selstring + if isinstance(seldef, string_types): + # if we have a single string + out = seldef + elif all([isinstance(i, int) for i in seldef]): + # if we have a single list of indices + out = np.array(seldef) + else: + out = [] + for item in seldef: + if isinstance(item, string_types): + out.append(item) + else: + out.append(np.array(item)) + + out = tuple(out) + + return out diff --git a/src/mdsynthesis/tests/test_filesystem.py b/src/mdsynthesis/tests/test_filesystem.py index 9e470a4..0cfdef0 100644 --- a/src/mdsynthesis/tests/test_filesystem.py +++ b/src/mdsynthesis/tests/test_filesystem.py @@ -28,8 +28,8 @@ def sim_external(self, tmpdir): py.path.local(GRO).copy(GRO_t) py.path.local(XTC).copy(XTC_t) - s.udef.topology = GRO_t.strpath - s.udef.trajectory = XTC_t.strpath + s.universedef.topology = GRO_t.strpath + s.universedef.trajectory = XTC_t.strpath return s @pytest.fixture @@ -45,8 +45,8 @@ def sim_internal(self, tmpdir): py.path.local(GRO).copy(GRO_t) py.path.local(XTC).copy(XTC_t) - s.udef.topology = GRO_t.strpath - s.udef.trajectory = XTC_t.strpath + s.universedef.topology = GRO_t.strpath + s.universedef.trajectory = XTC_t.strpath return s def test_move_Sim_external(self, sim_external, tmpdir): @@ -57,16 +57,16 @@ def test_move_Sim_external(self, sim_external, tmpdir): assert isinstance(sim_external.universe, MDAnalysis.Universe) sim = sim_external - assert sim.universe.filename == sim.udef.topology - assert sim.universe.trajectory.filename == sim.udef.trajectory + assert sim.universe.filename == sim.universedef.topology + assert sim.universe.trajectory.filename == sim.universedef.trajectory def test_move_Sim_internal(self, sim_internal, tmpdir): sim_internal.location = tmpdir.mkdir('test').strpath assert isinstance(sim_internal.universe, MDAnalysis.Universe) sim = sim_internal - assert sim.universe.filename == sim.udef.topology - assert sim.universe.trajectory.filename == sim.udef.trajectory + assert sim.universe.filename == sim.universedef.topology + assert sim.universe.trajectory.filename == sim.universedef.trajectory def test_move_Sim_internal_copy(self, sim_internal, tmpdir): """We move the whole Sim, including its internal trajectory files, and @@ -74,7 +74,7 @@ def test_move_Sim_internal_copy(self, sim_internal, tmpdir): be. """ grofile = tmpdir.join(sim_internal.name, 'sub', self.GRO) - assert sim_internal.udef.topology == grofile.strpath + assert sim_internal.universedef.topology == grofile.strpath sim_internal.location = tmpdir.mkdir('test').strpath tmpdir.join('test', sim_internal.name, 'sub', self.GRO).copy( diff --git a/src/mdsynthesis/tests/test_treants.py b/src/mdsynthesis/tests/test_treants.py index 6970c2a..e3877f4 100644 --- a/src/mdsynthesis/tests/test_treants.py +++ b/src/mdsynthesis/tests/test_treants.py @@ -9,6 +9,8 @@ import os import shutil import py +from pkg_resources import parse_version + from datreant.core.tests.test_treants import TestTreant import MDAnalysis as mda @@ -35,8 +37,8 @@ class TestUniverse: def test_add_universe(self, treant): """Test adding a new unverse definition""" - treant.udef.topology = GRO - treant.udef.trajectory = XTC + treant.universedef.topology = GRO + treant.universedef.trajectory = XTC assert isinstance(treant.universe, mda.Universe) @@ -52,8 +54,8 @@ def test_set_universe(self, treant): treant.universe = u assert isinstance(treant.universe, mda.Universe) - assert treant.udef.topology == GRO - assert treant.udef.trajectory == XTC + assert treant.universedef.topology == GRO + assert treant.universedef.trajectory == XTC def test_set_universe_only_topology(self, treant): """Test setting the Universe to a topology-only universe""" @@ -62,18 +64,21 @@ def test_set_universe_only_topology(self, treant): treant.universe = u assert treant.universe.filename == PSF - assert treant.udef.topology == PSF - assert treant.udef.trajectory is None + assert treant.universedef.topology == PSF + assert treant.universedef.trajectory is None def test_set_universe_chainreader(self, treant): """Test setting the Universe to multiple file trajectory (chain)""" - u = mda.Universe(GRO, [XTC, XTC]) + u = mda.Universe(GRO, (XTC, XTC)) treant.universe = u - assert treant.udef.topology == GRO - assert treant.udef.trajectory == [XTC, XTC] + assert treant.universedef.topology == GRO + assert treant.universedef.trajectory == (XTC, XTC) + @pytest.mark.skipif((parse_version(mda.__version__) < + parse_version('0.15.0')), + reason="requires MDAnalysis >= 0.15.0") def test_set_universe_with_kwargs(self, treant): """Universe should preserve its kwargs, if possible. @@ -87,7 +92,7 @@ def test_set_universe_with_kwargs(self, treant): u = mda.Universe(PDB, XTC, something_fake=True) treant.universe = u - assert treant.udef.kwargs['something_fake'] is True + assert treant.universedef.kwargs['something_fake'] is True # try a kwarg with a value that won't be serializable u2 = mda.Universe(PDB, XTC, @@ -109,42 +114,42 @@ def test_add_univese_typeerror(self, treant): def test_remove_universe(self, treant): """Test universe removal""" - treant.udef.topology = GRO - treant.udef.trajectory = XTC + treant.universedef.topology = GRO + treant.universedef.trajectory = XTC assert isinstance(treant.universe, mda.Universe) - treant.udef.topology = None - assert treant.udef.topology is None + treant.universedef.topology = None + assert treant.universedef.topology is None assert treant.universe is None # trajectory definition should still be there - assert treant.udef.trajectory + assert treant.universedef.trajectory # this should give us a universe again - treant.udef.topology = PDB + treant.universedef.topology = PDB assert isinstance(treant.universe, mda.Universe) # removing trajectories should keep universe, but with PDB as # coordinates - treant.udef.trajectory = None + treant.universedef.trajectory = None assert isinstance(treant.universe, mda.Universe) assert treant.universe.trajectory.n_frames == 1 def test_set_resnums(self, treant): """Test that we can add resnums to a universe.""" - treant.udef.topology = GRO - treant.udef.trajectory = XTC + treant.universedef.topology = GRO + treant.universedef.trajectory = XTC protein = treant.universe.select_atoms('protein') resids = protein.residues.resids protein.residues.set_resnum(resids + 3) - treant.udef._set_resnums(treant.universe.residues.resnums) + treant.universedef._set_resnums(treant.universe.residues.resnums) - treant.udef.reload() + treant.universedef.reload() protein = treant.universe.select_atoms('protein') assert (resids + 3 == protein.residues.resnums).all() @@ -153,9 +158,9 @@ def test_set_resnums(self, treant): protein.residues.set_resnum(resids + 6) assert (protein.residues.resnums == resids + 6).all() - treant.udef._set_resnums(treant.universe.residues.resnums) + treant.universedef._set_resnums(treant.universe.residues.resnums) - treant.udef.reload() + treant.universedef.reload() protein = treant.universe.select_atoms('protein') assert (resids + 6 == protein.residues.resnums).all() @@ -167,8 +172,8 @@ def treant(self, tmpdir): with tmpdir.as_cwd(): s = mds.Sim(TestSim.treantname) - s.udef.topology = GRO - s.udef.trajectory = XTC + s.universedef.topology = GRO + s.universedef.trajectory = XTC return s def test_add_selection(self, treant): @@ -180,9 +185,10 @@ def test_add_selection(self, treant): CA = treant.universe.select_atoms('protein and name CA') someres = treant.universe.select_atoms('resid 12') - assert (CA.indices == treant.atomselections['CA'].indices).all() + assert (CA.indices == + treant.atomselections.create('CA').indices).all() assert (someres.indices == - treant.atomselections['someres'].indices).all() + treant.atomselections.create('someres').indices).all() def test_remove_selection(self, treant): """Test universe removal""" @@ -217,13 +223,14 @@ def test_selection_keys(self, treant): assert set(('CA', 'someres')) == set(treant.atomselections.keys()) - def test_selection_define(self, treant): + def test_selection_get(self, treant): CA = 'protein and name CA' treant.atomselections.add('CA', CA) - assert treant.atomselections.define('CA')[0] == CA + assert treant.atomselections.get('CA') == CA + assert treant.atomselections['CA'] == CA - def test_selection_get(self, treant): + def test_selection_get_not_present(self, treant): with pytest.raises(KeyError): treant.atomselections['CA'] @@ -233,7 +240,7 @@ def test_add_atomselections_multiple_strings_via_add(self, treant): assert 'funky town' in treant.atomselections ref = treant.universe.select_atoms('name N', 'name CA') - sel = treant.atomselections['funky town'] + sel = treant.atomselections.create('funky town') assert (ref.indices == sel.indices).all() def test_add_atomselections_multiple_strings_via_setitem(self, treant): @@ -242,29 +249,57 @@ def test_add_atomselections_multiple_strings_via_setitem(self, treant): assert 'funky town 2' in treant.atomselections ref = treant.universe.select_atoms('name N', 'name CA') - sel = treant.atomselections['funky town 2'] + sel = treant.atomselections.create('funky town 2') assert (ref.indices == sel.indices).all() - def test_add_selection_as_atomgroup_via_add(self, treant): + def test_add_selection_as_indices_via_add(self, treant): """Make an arbitrary AtomGroup then save selection as AG""" ag = treant.universe.atoms[:10:2] - treant.atomselections.add('ag sel', ag) + treant.atomselections.add('ag sel', ag.indices) assert 'ag sel' in treant.atomselections - ag2 = treant.atomselections['ag sel'] + ag2 = treant.atomselections.create('ag sel') assert (ag.indices == ag2.indices).all() - def test_add_selection_as_atomgroup_via_setitem(self, treant): + def test_add_selection_as_indices_via_setitem(self, treant): """Make an arbitrary AtomGroup then save selection as AG""" ag = treant.universe.atoms[25:50:3] - treant.atomselections['ag sel 2'] = ag + treant.atomselections['ag sel 2'] = ag.indices assert 'ag sel 2' in treant.atomselections - ag2 = treant.atomselections['ag sel 2'] + ag2 = treant.atomselections.create('ag sel 2') assert (ag.indices == ag2.indices).all() + def test_add_selection_as_mix_via_setitem(self, treant): + """Save a selection as a mixture of atom indices and strings""" + ag = treant.universe.atoms[25:50:3] + + treant.atomselections['ag sel 2'] = ('protein and name CA', + ag.indices) + + assert 'ag sel 2' in treant.atomselections + + ag2 = treant.atomselections.create('ag sel 2') + + ag3 = treant.universe.select_atoms('protein and name CA') + ag + assert (ag2.indices == ag3.indices).all() + + def test_add_selection_as_mix_via_add(self, treant): + """Save a selection as a mixture of atom indices and strings""" + ag = treant.universe.atoms[25:50:3] + + treant.atomselections.add('ag sel 2', 'protein and name CA', + ag.indices) + + assert 'ag sel 2' in treant.atomselections + + ag2 = treant.atomselections.create('ag sel 2') + + ag3 = treant.universe.select_atoms('protein and name CA') + ag + assert (ag2.indices == ag3.indices).all() + class TestReadOnly: """Test Sim functionality when read-only""" @@ -284,8 +319,8 @@ def sim(self, tmpdir, request): py.path.local(GRO).copy(GRO_t) py.path.local(XTC).copy(XTC_t) - c.udef.topology = GRO_t.strpath - c.udef.trajectory = XTC_t.strpath + c.universedef.topology = GRO_t.strpath + c.universedef.trajectory = XTC_t.strpath py.path.local(c.abspath).chmod(0550, rec=True) diff --git a/src/mdsynthesis/treants.py b/src/mdsynthesis/treants.py index 202f8a8..f087fae 100644 --- a/src/mdsynthesis/treants.py +++ b/src/mdsynthesis/treants.py @@ -54,7 +54,7 @@ def __init__(self, sim, new=False, categories=None, tags=None): categories=categories, tags=tags) - self._udef = None + self._universedef = None self._atomselections = None self._universe = None # universe 'dock' @@ -78,35 +78,35 @@ def universe(self): if self._universe: return self._universe else: - self.udef._activate() + self.universedef._activate() return self._universe @universe.setter def universe(self, universe): if universe is None: - self.udef.topology = None - self.udef.trajectory = None - self.udef.kwargs = None - self._universe = None + self.universedef._set_topology(None) + self.universedef._set_trajectory([]) + self.universedef.kwargs = None + self.universedef.activate() elif not isinstance(universe, Universe): raise TypeError("Cannot set to {}; must be Universe".format( type(universe))) else: - self.udef.topology = universe.filename + self.universedef._set_topology(universe.filename) try: # ChainReader? traj = universe.trajectory.filenames except AttributeError: try: # Reader? - traj = universe.trajectory.filename + traj = [universe.trajectory.filename] except AttributeError: # Only topology - traj = None + traj = [] - self.udef.trajectory = traj + self.universedef._set_trajectory(traj) # try and store keyword arguments try: - self.udef.kwargs = universe._kwargs + self.universedef.kwargs = universe.kwargs except AttributeError: warnings.warn("Universe did not keep keyword arguments; " "cannot store keyword arguments for Universe.") @@ -114,17 +114,24 @@ def universe(self, universe): # finally, just use this instance self._universe = universe + @universe.deleter + def universe(self): + self.universedef._set_topology(None) + self.universedef._set_trajectory([]) + self.universedef.kwargs = None + self.universedef.activate() + @property - def udef(self): + def universedef(self): """The universe definition for this Sim. """ # attach universe if not attached, and only give results if a # universe is present thereafter - if not self._udef: - self._udef = limbs.UniverseDefinition(self) + if not self._universedef: + self._universedef = limbs.UniverseDefinition(self) - return self._udef + return self._universedef @property def atomselections(self): From 89b3ca0dc82c8d8dd26e666dac0911d03528b06f Mon Sep 17 00:00:00 2001 From: David Dotson Date: Mon, 4 Apr 2016 16:41:44 -0700 Subject: [PATCH 7/7] Updated docs to be in line with new API changes. --- docs/sims.rst | 77 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 28 deletions(-) diff --git a/docs/sims.rst b/docs/sims.rst index 7f05348..57a9102 100644 --- a/docs/sims.rst +++ b/docs/sims.rst @@ -46,38 +46,53 @@ access the Sim's Universe directly with:: >>> s.universe -At this point, we get back ``None``. However, if we define a topology for the -Universe with +At this point, we get back ``None``. However, we can define the Sim's Universe +by giving it one direclty:: - >>> s.udef.topology = 'path/to/topology.psf' + >>> import MDAnalysis as mda + >>> s.universe = mda.Universe('path/to/topology.psf', + 'path/to/trajectory.dcd') + >>> s.universe + -then we get back a Universe built with this topology instead:: +The Universe definition is persistent, so we can get back an identical Universe +later from another Python session with our Sim:: + >>> import mdsynthesis as mds + >>> s = mds.Sim('adk') >>> s.universe -We can also add a trajectory:: - >>> s.udef.trajectory = 'path/to/trajectory.dcd' +Changing the Universe definition +-------------------------------- +.. warning:: This interface may be removed in a future release, but remains for + now due to limitations in MDAnalysis. It is encouraged to set the + Universe definition directly as shown above. + +We can directly change the topology used for the Sim's Universe with :: + + >>> s.universedef.topology = 'path/to/another/topology.psf' + +then we get back a Universe built with this topology instead:: + + >>> s.universe.filename + '/home/bob/research/path/to/another/topology.psf' + +We can also change the trajectory:: + + >>> s.universedef.trajectory = 'path/to/another/trajectory.dcd' which re-initializes the Universe with both the defined topology and trajectory:: >>> s.universe.trajectory - + We can also define our Universe as having multiple trajectories by giving a list of filepaths instead. Internally, the Universe generated will use the MDAnalysis :class:`~MDAnalysis.coordinates.base.ChainReader` for treating the trajectories as a contiguous whole. -The Universe definition is persistent, so we can get back an identical Universe -later from another Python session with our Sim:: - - >>> import mdsynthesis as mds - >>> s = mds.Sim('adk') - >>> s.universe - - .. note:: Changing the topology or trajectory definition will reload the Universe automatically. This means that any AtomGroups you are working with will not point to the new Universe, but perhaps the old @@ -89,14 +104,14 @@ If the Universe needed requires keyword arguments on initialization, these can be stored as well. For example, if our topology was a PDB file and we wanted bonds to be guessed upfront, we could make this happen every time:: - >>> s.udef.kwargs = {'guess_bonds': True} + >>> s.universedef.kwargs = {'guess_bonds': True} Reinitializing the Universe --------------------------- If you make modifications to the Universe but you want to restore the original from its definition, you can force it to reload with:: - >>> s.udef.reload() + >>> s.universedef.reload() API Reference: UniverseDefinition --------------------------------- @@ -120,14 +135,15 @@ kinase, the protein we simulated. We can store these immediately:: >>> s.atomselections['lid'] = 'resid 122:159' >>> s.atomselections['nmp'] = 'resid 30:59' - >>> s.atomselections['core'] = ['resid 1:29', 'resid 60:121', 'resid 160:214'] + >>> s.atomselections['core'] = ('resid 1:29', 'resid 60:121', 'resid 160:214') -We can now get new AtomGroups back for each selection at any time :: +We can now get new AtomGroups back for each selection at any time with the +:meth:`~mdsynthesis.limbs.AtomSelections.create` method:: - >>> s.atomselections['lid'] + >>> s.atomselections.create('lid') - >>> s.atomselections['core'] + >>> s.atomselections.create('core') and we don't have to remember or know how 'lid' or 'core' are defined for this @@ -145,13 +161,14 @@ work with many variants of a simulation system without having to micromanage. alignments. Want just the selection strings back? We can use -:meth:`~mdsynthesis.limbs.AtomSelections.define`:: +:meth:`~mdsynthesis.limbs.AtomSelections.get`:: - >>> s.atomselections.define('lid') - ['resid 122:159'] + >>> s.atomselections.get('lid') + 'resid 122:159' -Note that selections are always stored as lists, even if only a single -selection string was given. + # or using getitem syntax + >>> s.atomselections['lid'] + 'resid 122:159' Atom selections from atom indices --------------------------------- @@ -159,10 +176,14 @@ Do you already have an AtomGroup and prefer to define it according to its atom indices instead of as a selection string? That can be done, too:: >>> lid = s.universe.select_atoms('resid 122:159') - >>> s.atomselections['lid'] = lid - >>> s.atomselections['lid'] + >>> s.atomselections['lid'] = lid.indices + >>> s.atomselections.create('lid') +Lists/tuples of selection strings or atom indices can be stored in any combination +as a selection. These are applied in order to yield the AtomGroup when calling the +:meth:`~mdsynthesis.limbs.AtomSelections.create` method. + API Reference: AtomSelections ----------------------------- See the :ref:`AtomSelections_api` API reference for more details.