Skip to content

Commit

Permalink
Fix/deep copy issue 153 based on issue 168 (#207)
Browse files Browse the repository at this point in the history
* Implement unit tests catching issue #168.

* Refactor state point management in Job class.

And explicitly check key type in SyncedDict.

* Remove obsolete SPDict class from job module.

* Test the implicit tuple to list conversion.

* Refactor the `Job.reset_statepoint()` function for clarity.

* Use property instead of internal variable.

* Refactor 'reset_document' function into JSONDict class.

The reset function for the Job.document and Project.document is thereby
implemented as part of the JSONDict class.

* Convert mappings upon reset (JSONDict).

* Refactor Job constructor.

* Implement unit tests that catch issue with deep-copying a state point.

Modifying a deep copy of a job state point should not change the job's
actual state point.

* Committing a fix for now, awaiting for further evaluation of expected behavior

* Update state point copy/deepcopy behavior to be essentially opposite.

  * copy:
      Copy the statepoint, but keep reference to original job,
      the original job is modified as we modify the state point.
  * deepcopy:
      Create a deep copy of the statepoint, with a new job instance,
      the original job instance is left intact as we modify the
      copied state point. This means the original job is no longer in
      the project upon state point modification.

* Update test_interface_(deep)copy unit tests.

* Implement copy/deepcopy for instances of Job.

And add related unit tests.

* Need to explicitly implement Job.__deepcopy__.

* Update changelog.
  • Loading branch information
csadorf committed Jul 19, 2019
1 parent 09a84d0 commit f83f86d
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 4 deletions.
22 changes: 18 additions & 4 deletions signac/contrib/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import errno
import logging
import shutil
from copy import deepcopy

from ..common import six
from ..core import json
Expand All @@ -22,14 +23,15 @@

class _sp_save_hook(object):

def __init__(self, job):
self.job = job
def __init__(self, *jobs):
self.jobs = list(jobs)

def load(self):
pass

def save(self):
self.job._reset_sp()
for job in self.jobs:
job._reset_sp()


class Job(object):
Expand Down Expand Up @@ -144,7 +146,7 @@ def reset_statepoint(self, new_statepoint):
else:
raise
# Update this instance
self._statepoint = SyncedAttrDict(dst._statepoint._as_dict(), parent=_sp_save_hook(self))
self._statepoint._data = dst._statepoint._data
self._id = dst._id
self._wd = dst._wd
self._fn_doc = dst._fn_doc
Expand Down Expand Up @@ -579,3 +581,15 @@ def __enter__(self):
def __exit__(self, err_type, err_value, tb):
self.close()
return False

def __setstate__(self, state):
self.__dict__.update(state)
self._statepoint._parent.jobs.append(self)

def __deepcopy__(self, memo):
cls = self.__class__
result = cls.__new__(cls)
memo[id(self)] = result
for k, v in self.__dict__.items():
setattr(result, k, deepcopy(v, memo))
return result
104 changes: 104 additions & 0 deletions tests/test_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,80 @@ def test_isfile(self):
file.write('hello')
self.assertTrue(job.isfile(fn))

def test_copy(self):
job = self.project.open_job({'a': 0}).init()
self.assertIn(job, self.project)

# Modify copy
copied_job = copy.copy(job)
self.assertFalse(job is copied_job)
self.assertEqual(job, copied_job)
self.assertEqual(job.sp, copied_job.sp)
self.assertIn(job, self.project)
self.assertIn(copied_job, self.project)
copied_job.sp.a = 1
self.assertIn(job, self.project)
self.assertIn(copied_job, self.project)
self.assertEqual(job, copied_job)
self.assertEqual(job.sp, copied_job.sp)

# Modify original
copied_job = copy.copy(job)
self.assertFalse(job is copied_job)
self.assertEqual(job, copied_job)
self.assertEqual(job.sp, copied_job.sp)
self.assertIn(job, self.project)
self.assertIn(copied_job, self.project)
job.sp.a = 2
self.assertIn(job, self.project)
self.assertIn(copied_job, self.project)
self.assertEqual(job, copied_job)
self.assertEqual(job.sp, copied_job.sp)

# Delete original
del job
self.assertIn(copied_job, self.project)
copied_job.sp.a = 3
self.assertIn(copied_job, self.project)

def test_deepcopy(self):
job = self.project.open_job({'a': 0}).init()
self.assertIn(job, self.project)

# Modify copy
copied_job = copy.deepcopy(job)
self.assertFalse(job is copied_job)
self.assertEqual(job, copied_job)
self.assertEqual(job.sp, copied_job.sp)
self.assertIn(job, self.project)
self.assertIn(copied_job, self.project)
copied_job.sp.a = 1
self.assertNotIn(job, self.project)
self.assertIn(copied_job, self.project)
self.assertNotEqual(job, copied_job)
self.assertNotEqual(job.sp, copied_job.sp)

# Modify original
job = self.project.open_job({'a': 0}).init()
copied_job = copy.deepcopy(job)
self.assertFalse(job is copied_job)
self.assertEqual(job, copied_job)
self.assertEqual(job.sp, copied_job.sp)
self.assertIn(job, self.project)
self.assertIn(copied_job, self.project)
job.sp.a = 2
self.assertIn(job, self.project)
self.assertNotIn(copied_job, self.project)
self.assertNotEqual(job, copied_job)
self.assertNotEqual(job.sp, copied_job.sp)

# Delete original
copied_job = copy.deepcopy(job)
del job
self.assertIn(copied_job, self.project)
copied_job.sp.a = 3
self.assertIn(copied_job, self.project)


class JobSPInterfaceTest(BaseJobTest):

Expand Down Expand Up @@ -252,6 +326,16 @@ def test_interface_rename(self):
self.assertNotIn('a', job.sp)
self.assertEqual(job.sp.b, 0)

def test_interface_copy(self):
job = self.open_job(dict(a=0)).init()
copy.copy(job.sp).a = 1
self.assertTrue(job in self.project)

def test_interface_deepcopy(self):
job = self.open_job(dict(a=0)).init()
copy.deepcopy(job.sp).a = 1
self.assertFalse(job in self.project)

def test_interface_add(self):
job = self.open_job(dict(a=0))
job.init()
Expand Down Expand Up @@ -1447,6 +1531,26 @@ def test_update_statepoint(self):
self.assertIn(key, dst2_job.data)
self.assertEqual(len(dst2_job.data), 1)

def test_statepoint_copy(self):
job = self.open_job(dict(a=test_token, b=test_token)).init()
_id = job.get_id()
sp_copy = copy.copy(job.sp)
del sp_copy['b']
self.assertIn('a', job.sp)
self.assertNotIn('b', job.sp)
self.assertIn(job, self.project)
self.assertNotEqual(job.get_id(), _id)

def test_statepoint_deepcopy(self):
job = self.open_job(dict(a=test_token, b=test_token)).init()
_id = job.get_id()
sp_copy = copy.deepcopy(job.sp)
del sp_copy['b']
self.assertIn('a', job.sp)
self.assertIn('b', job.sp)
self.assertNotIn(job, self.project)
self.assertEqual(job.get_id(), _id)


@unittest.skipIf(not H5PY, 'test requires the h5py package')
class JobClosedDataTest(JobOpenDataTest):
Expand Down

0 comments on commit f83f86d

Please sign in to comment.