Skip to content

Commit

Permalink
Remove deepcopy (#694)
Browse files Browse the repository at this point in the history
* Remove deepcopy

Just found this one after updating something internal and hitting serialization issue again. See previous PR regarding deepcopy (they have no purpose).

* Fixed flake8 issues

* Add deepcopy to test

Engine input isolation removed, update test with deepcopy.

* Isolation no longer expected

Remove test with this expectation.

* Reconfigure tests no isolation nb.

Used deepcopy where necessary when tests used the isolated nb
expectation in unittest logic for assertion checks.

Co-authored-by: Matthew Seal <mseal007@gmail.com>
  • Loading branch information
surdouski and MSeal authored Oct 18, 2022
1 parent fdf4880 commit 54f6c03
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 14 deletions.
4 changes: 1 addition & 3 deletions papermill/engines.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"""Engines to perform different roles"""
import sys
import copy
import datetime
import dateutil

Expand Down Expand Up @@ -99,8 +98,7 @@ class NotebookExecutionManager(object):
def __init__(
self, nb, output_path=None, log_output=False, progress_bar=True, autosave_cell_every=30
):
# Deep copy the input to isolate the object being executed against
self.nb = copy.deepcopy(nb)
self.nb = nb
self.output_path = output_path
self.log_output = log_output
self.start_time = None
Expand Down
20 changes: 9 additions & 11 deletions papermill/tests/test_engines.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,6 @@ def setUp(self):
self.foo_nb = copy.deepcopy(self.nb)
self.foo_nb.metadata['foo'] = 'bar'

def test_nb_isolation(self):
"""
Tests that the engine notebook is isolated from source notebook
"""
nb_man = NotebookExecutionManager(self.nb)
nb_man.nb.metadata['foo'] = 'bar'
self.assertNotEqual(nb_man.nb, self.nb)

def test_basic_pbar(self):
nb_man = NotebookExecutionManager(self.nb)

Expand Down Expand Up @@ -343,7 +335,11 @@ def execute_managed_notebook(cls, nb_man, kernel_name, **kwargs):
nb_man.cell_complete(cell)

with patch.object(NotebookExecutionManager, 'save') as save_mock:
nb = CellCallbackEngine.execute_notebook(self.nb, 'python', output_path='foo.ipynb')
nb = CellCallbackEngine.execute_notebook(
copy.deepcopy(self.nb),
'python',
output_path='foo.ipynb'
)

self.assertEqual(nb, AnyMock(NotebookNode))
self.assertNotEqual(self.nb, nb)
Expand Down Expand Up @@ -373,7 +369,9 @@ def execute_managed_notebook(cls, nb_man, kernel_name, **kwargs):
with patch.object(NotebookExecutionManager, 'save') as save_mock:
with patch.object(NotebookExecutionManager, 'complete_pbar') as pbar_comp_mock:
nb = NoCellCallbackEngine.execute_notebook(
self.nb, 'python', output_path='foo.ipynb'
copy.deepcopy(self.nb),
'python',
output_path='foo.ipynb'
)

self.assertEqual(nb, AnyMock(NotebookNode))
Expand Down Expand Up @@ -407,7 +405,7 @@ def test_nb_convert_engine(self):
with patch.object(engines, 'PapermillNotebookClient') as client_mock:
with patch.object(NotebookExecutionManager, 'save') as save_mock:
nb = NBClientEngine.execute_notebook(
self.nb,
copy.deepcopy(self.nb),
'python',
output_path='foo.ipynb',
progress_bar=False,
Expand Down

0 comments on commit 54f6c03

Please sign in to comment.