From cd95045c7b0a4b176b5e66cdd9c3cb911fb08b9c Mon Sep 17 00:00:00 2001 From: Jan Freyberg Date: Sun, 2 Dec 2018 15:41:37 +0000 Subject: [PATCH 1/8] factor out notebook execution --- faculty_mill/execute.py | 31 +++++++++++++++++++++++++++++++ faculty_mill/faculty_reporter.py | 19 ++----------------- 2 files changed, 33 insertions(+), 17 deletions(-) create mode 100644 faculty_mill/execute.py diff --git a/faculty_mill/execute.py b/faculty_mill/execute.py new file mode 100644 index 0000000..0cbf82a --- /dev/null +++ b/faculty_mill/execute.py @@ -0,0 +1,31 @@ +import shutil +from pathlib import Path +from io import TextIOWrapper + +import click +from papermill.cli import papermill + + +def run( + notebook: TextIOWrapper, + directory: Path, + execute: bool, + click_context: click.Context, +) -> Path: + input_path = directory / "input.ipynb" + output_path = directory / "output.ipynb" + with input_path.open("w") as input_file: + shutil.copyfileobj(notebook, input_file) + + if execute: + papermill_click_context = papermill.make_context( + "The papermill execution command.", + [str(input_path), str(output_path)] + click_context.args, + parent=click_context, + ) + + papermill.invoke(papermill_click_context) + else: + shutil.copy(input_path, output_path) + + return output_path diff --git a/faculty_mill/faculty_reporter.py b/faculty_mill/faculty_reporter.py index ed00287..b04f481 100644 --- a/faculty_mill/faculty_reporter.py +++ b/faculty_mill/faculty_reporter.py @@ -12,6 +12,7 @@ from papermill.cli import papermill from .version import print_version +from .execute import run @contextmanager @@ -77,23 +78,7 @@ def main( with tmpdir() as directory: directory = Path(directory) - input_path = directory / "input.ipynb" - output_path = directory / "output.ipynb" - - # write the input file to the new file - with input_path.open("w") as input_file: - shutil.copyfileobj(notebook, input_file) - - if execute: - papermill_click_context = papermill.make_context( - "The papermill execution command.", - [str(input_path), str(output_path)] + click_context.args, - parent=click_context, - ) - - papermill.invoke(papermill_click_context) - else: - shutil.copy(input_path, output_path) + output_path = run(notebook, directory, execute, click_context) reports = { report.name: report for report in report_client.list(PROJECT_ID) From 5556d1423df7b368ef73b28c20f790edb68070b7 Mon Sep 17 00:00:00 2001 From: Jan Freyberg Date: Sun, 2 Dec 2018 15:41:55 +0000 Subject: [PATCH 2/8] ignore coverage-related files --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index ef8bc3d..c661ff5 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,5 @@ dist/ .pytest_cache/ __pycache__/ *.pyc +.coverage +htmlcov/ From 6c8fcf3cca3cd77fd4c627c0623f02a28265f676 Mon Sep 17 00:00:00 2001 From: Jan Freyberg Date: Sun, 2 Dec 2018 15:42:58 +0000 Subject: [PATCH 3/8] test the new execute file --- tests/test_execute.py | 55 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 tests/test_execute.py diff --git a/tests/test_execute.py b/tests/test_execute.py new file mode 100644 index 0000000..5e8c5ca --- /dev/null +++ b/tests/test_execute.py @@ -0,0 +1,55 @@ +from io import StringIO +from pathlib import Path +from unittest.mock import Mock, patch +import pytest + +from faculty_mill.execute import run + + +@pytest.fixture +def tempdir(tmpdir): + """Fixture that wraps pytest fixture to return Path object""" + return Path(tmpdir) + + +@pytest.fixture +def tmpnotebook(): + test_notebook = StringIO() + test_notebook.write("test notebook content") + test_notebook.seek(0) + return test_notebook + + +@pytest.fixture +def mock_click_context(): + click_context = Mock() + click_context.args = ["arg 1", "arg 2"] + return click_context + + +def test_that_run_copies_content(tempdir, tmpnotebook, mock_click_context): + # tmpdir = Path(tmpdir) + output_notebook = run( + tmpnotebook, tempdir, execute=False, click_context=mock_click_context + ) + assert output_notebook.parent == tempdir + assert output_notebook.read_text() == "test notebook content" + + +def test_that_run_calls_papermill(tempdir, tmpnotebook, mock_click_context): + # NB: tmpdir is a pytest fixture + with patch("faculty_mill.execute.papermill") as mock_papermill: + output_notebook = run( + tmpnotebook, + tempdir, + execute=True, + click_context=mock_click_context, + ) + assert output_notebook.parent == tempdir + mock_papermill.make_context.assert_called_once_with( + "The papermill execution command.", + [str(tempdir / "input.ipynb"), str(tempdir / "output.ipynb")] + + mock_click_context.args, + parent=mock_click_context, + ) + mock_papermill.invoke.assert_called_once() From 7830208d7adc35f1e50c9114622f61cd04af9db4 Mon Sep 17 00:00:00 2001 From: Jan Freyberg Date: Sun, 2 Dec 2018 15:46:34 +0000 Subject: [PATCH 4/8] Remove unused imports --- faculty_mill/faculty_reporter.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/faculty_mill/faculty_reporter.py b/faculty_mill/faculty_reporter.py index b04f481..f8f0719 100644 --- a/faculty_mill/faculty_reporter.py +++ b/faculty_mill/faculty_reporter.py @@ -1,5 +1,4 @@ import os -import shutil from contextlib import contextmanager from pathlib import Path from tempfile import TemporaryDirectory @@ -9,7 +8,6 @@ import click import sherlockml import sml.auth -from papermill.cli import papermill from .version import print_version from .execute import run From 6d1843d88aab1a37e30e10bf657317175b75f833 Mon Sep 17 00:00:00 2001 From: Jan Freyberg Date: Sun, 2 Dec 2018 15:51:07 +0000 Subject: [PATCH 5/8] Fix error with tempdir fixture --- tests/test_execute.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_execute.py b/tests/test_execute.py index 5e8c5ca..3c3be9f 100644 --- a/tests/test_execute.py +++ b/tests/test_execute.py @@ -9,7 +9,7 @@ @pytest.fixture def tempdir(tmpdir): """Fixture that wraps pytest fixture to return Path object""" - return Path(tmpdir) + return Path(str(tmpdir)) @pytest.fixture From f3b5bfdd7e1e39025befbf42306567efc2f75da9 Mon Sep 17 00:00:00 2001 From: Jan Freyberg Date: Sun, 2 Dec 2018 15:59:36 +0000 Subject: [PATCH 6/8] Fix a python 3.5 compatibility error --- faculty_mill/execute.py | 2 +- tests/test_execute.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/faculty_mill/execute.py b/faculty_mill/execute.py index 0cbf82a..dd513d2 100644 --- a/faculty_mill/execute.py +++ b/faculty_mill/execute.py @@ -26,6 +26,6 @@ def run( papermill.invoke(papermill_click_context) else: - shutil.copy(input_path, output_path) + shutil.copy(str(input_path), str(output_path)) return output_path diff --git a/tests/test_execute.py b/tests/test_execute.py index 3c3be9f..6a58ecb 100644 --- a/tests/test_execute.py +++ b/tests/test_execute.py @@ -28,7 +28,6 @@ def mock_click_context(): def test_that_run_copies_content(tempdir, tmpnotebook, mock_click_context): - # tmpdir = Path(tmpdir) output_notebook = run( tmpnotebook, tempdir, execute=False, click_context=mock_click_context ) @@ -37,7 +36,6 @@ def test_that_run_copies_content(tempdir, tmpnotebook, mock_click_context): def test_that_run_calls_papermill(tempdir, tmpnotebook, mock_click_context): - # NB: tmpdir is a pytest fixture with patch("faculty_mill.execute.papermill") as mock_papermill: output_notebook = run( tmpnotebook, @@ -52,4 +50,6 @@ def test_that_run_calls_papermill(tempdir, tmpnotebook, mock_click_context): + mock_click_context.args, parent=mock_click_context, ) - mock_papermill.invoke.assert_called_once() + mock_papermill.invoke.assert_called_once_with( + mock_papermill.make_context.return_value + ) From 03e748ac588941d262bd24d935deefa10031acc6 Mon Sep 17 00:00:00 2001 From: Jan Freyberg Date: Tue, 4 Dec 2018 14:18:04 +0000 Subject: [PATCH 7/8] Using patch as decorator instead of context manager --- tests/test_execute.py | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/tests/test_execute.py b/tests/test_execute.py index 6a58ecb..c1dba10 100644 --- a/tests/test_execute.py +++ b/tests/test_execute.py @@ -35,21 +35,20 @@ def test_that_run_copies_content(tempdir, tmpnotebook, mock_click_context): assert output_notebook.read_text() == "test notebook content" -def test_that_run_calls_papermill(tempdir, tmpnotebook, mock_click_context): - with patch("faculty_mill.execute.papermill") as mock_papermill: - output_notebook = run( - tmpnotebook, - tempdir, - execute=True, - click_context=mock_click_context, - ) - assert output_notebook.parent == tempdir - mock_papermill.make_context.assert_called_once_with( - "The papermill execution command.", - [str(tempdir / "input.ipynb"), str(tempdir / "output.ipynb")] - + mock_click_context.args, - parent=mock_click_context, - ) - mock_papermill.invoke.assert_called_once_with( - mock_papermill.make_context.return_value - ) +@patch("faculty_mill.execute.papermill") +def test_that_run_calls_papermill( + mock_papermill, tempdir, tmpnotebook, mock_click_context +): + output_notebook = run( + tmpnotebook, tempdir, execute=True, click_context=mock_click_context + ) + assert output_notebook.parent == tempdir + mock_papermill.make_context.assert_called_once_with( + "The papermill execution command.", + [str(tempdir / "input.ipynb"), str(tempdir / "output.ipynb")] + + mock_click_context.args, + parent=mock_click_context, + ) + mock_papermill.invoke.assert_called_once_with( + mock_papermill.make_context.return_value + ) From b86a0eaa5e5f02659fbbc116cda0decf8b039476 Mon Sep 17 00:00:00 2001 From: Jan Freyberg Date: Tue, 4 Dec 2018 17:40:13 +0000 Subject: [PATCH 8/8] Update faculty_reporter.py --- faculty_mill/faculty_reporter.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/faculty_mill/faculty_reporter.py b/faculty_mill/faculty_reporter.py index 5a714d4..e889c99 100644 --- a/faculty_mill/faculty_reporter.py +++ b/faculty_mill/faculty_reporter.py @@ -4,8 +4,6 @@ import click -from papermill.cli import papermill - from .publish import publish from .version import print_version from .execute import run