Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A Universe now holds onto its initialization kwargs. #813

Merged
merged 8 commits into from
Apr 6, 2016
4 changes: 3 additions & 1 deletion package/CHANGELOG
Expand Up @@ -13,7 +13,8 @@ The rules for this file:
* release numbers follow "Semantic Versioning" http://semver.org

------------------------------------------------------------------------------
??/??/16 jandom, abhinavgupta94, orbeckst, kain88-de, hainm, jdetle, jbarnoud
??/??/16 jandom, abhinavgupta94, orbeckst, kain88-de, hainm, jdetle, jbarnoud,
dotsdl

* 0.15.0

Expand All @@ -32,6 +33,7 @@ API Changes
Enhancements

* Add conda build scripts (Issue #608)
* Added read-only property giving Universe init kwargs (Issue #292)

Fixes

Expand Down
11 changes: 11 additions & 0 deletions package/MDAnalysis/core/AtomGroup.py
Expand Up @@ -3966,6 +3966,10 @@ def __init__(self, *args, **kwargs):
from ..topology.base import TopologyReader
from ..coordinates.base import ProtoReader

# hold on to copy of kwargs; used by external libraries that
# reinitialize universes
self._kwargs = copy.deepcopy(kwargs)

# managed attribute holding Reader
self._trajectory = None

Expand Down Expand Up @@ -4262,6 +4266,13 @@ def universe(self):
# which might be undesirable if it has a __del__ method. It is also cleaner than a weakref.
return self

@property
def kwargs(self):
"""Keyword arguments used to initialize this universe (read-only).

"""
return copy.deepcopy(self._kwargs)

@property
@cached('fragments')
def fragments(self):
Expand Down
17 changes: 17 additions & 0 deletions testsuite/MDAnalysisTests/test_atomgroup.py
Expand Up @@ -1724,7 +1724,19 @@ def test_set_dimensions(self):
u.dimensions = np.array([10, 11, 12, 90, 90, 90])
assert_allclose(u.dimensions, box)

def test_universe_kwargs(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QC has a good point here making this a staticmethod

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we don't use self anywhere inside? Many of the other methods don't use self anywhere inside either.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like QC's rationale for making it @staticmethod:

"This will improve performance and make it clear to users of your code that the function does not use any attributes of the class."

We should do it when we can.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you also need to remove the self variable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Sorry!

u = MDAnalysis.Universe(PSF, PDB_small, fake_kwarg=True)
assert_equal(len(u.atoms), 3341, "Loading universe failed somehow")

assert_(u.kwargs['fake_kwarg'] is True)

# initialize new universe from pieces of existing one
u2 = MDAnalysis.Universe(u.filename, u.trajectory.filename,
**u.kwargs)

assert_(u2.kwargs['fake_kwarg'] is True)
assert_equal(u.kwargs, u2.kwargs)

class TestPBCFlag(TestCase):
@dec.skipif(parser_not_found('TRZ'),
'TRZ parser not available. Are you using python 3?')
Expand Down Expand Up @@ -2121,11 +2133,13 @@ def _check_universe(self, u):
assert_equal(len(u.atoms[3].bonds), 2)
assert_equal(len(u.atoms[4].bonds), 1)
assert_equal(len(u.atoms[5].bonds), 1)
assert_('guess_bonds' in u.kwargs)

def test_universe_guess_bonds(self):
"""Test that making a Universe with guess_bonds works"""
u = MDAnalysis.Universe(two_water_gro, guess_bonds=True)
self._check_universe(u)
assert_(u.kwargs['guess_bonds'] is True)

def test_universe_guess_bonds_no_vdwradii(self):
"""Make a Universe that has atoms with unknown vdwradii."""
Expand All @@ -2136,13 +2150,16 @@ def test_universe_guess_bonds_with_vdwradii(self):
u = MDAnalysis.Universe(two_water_gro_nonames, guess_bonds=True,
vdwradii=self.vdw)
self._check_universe(u)
assert_(u.kwargs['guess_bounds'] is True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bounds

assert_equal(self.vdw, u.kwargs['vdwradii'])

def test_universe_guess_bonds_off(self):
u = MDAnalysis.Universe(two_water_gro_nonames, guess_bonds=False)

assert_equal(len(u.bonds), 0)
assert_equal(len(u.angles), 0)
assert_equal(len(u.dihedrals), 0)
assert_(u.kwargs['guess_bounds'] is False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bounds


def _check_atomgroup(self, ag, u):
"""Verify that the AtomGroup made bonds correctly,
Expand Down