From 29db1610585f6ef7bc0c4550100c06f93be9ba31 Mon Sep 17 00:00:00 2001 From: Nikita Kodenko Date: Tue, 2 Jun 2020 22:32:37 +0700 Subject: [PATCH 1/6] run: disable directories for -m/-M flags --- dvc/command/run.py | 5 ++--- dvc/output/base.py | 3 +++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/dvc/command/run.py b/dvc/command/run.py index 6f56e754e9..08d6d6a03e 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -132,7 +132,7 @@ def add_parser(subparsers, parent_parser): "--metrics", action="append", default=[], - help="Declare output metric file or directory.", + help="Declare output metric file.", metavar="", ) run_parser.add_argument( @@ -140,8 +140,7 @@ def add_parser(subparsers, parent_parser): "--metrics-no-cache", action="append", default=[], - help="Declare output metric file or directory " - "(do not put into DVC cache).", + help="Declare output metric file " "(do not put into DVC cache).", metavar="", ) run_parser.add_argument( diff --git a/dvc/output/base.py b/dvc/output/base.py index 344774c2c1..9bbf0aafae 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -256,6 +256,9 @@ def save(self): ) return + if self.metric: + self.verify_metric() + assert not self.IS_DEPENDENCY if not self.changed(): From b17f77d844511ea01f4e51e0c2fb4d689e193a99 Mon Sep 17 00:00:00 2001 From: Nikita Kodenko Date: Tue, 2 Jun 2020 22:36:49 +0700 Subject: [PATCH 2/6] run: reformatted string literals --- dvc/command/run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/run.py b/dvc/command/run.py index 08d6d6a03e..a70ab74460 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -140,7 +140,7 @@ def add_parser(subparsers, parent_parser): "--metrics-no-cache", action="append", default=[], - help="Declare output metric file " "(do not put into DVC cache).", + help="Declare output metric file (do not put into DVC cache).", metavar="", ) run_parser.add_argument( From 3767296d18f42bc46684a93e5bd52e631a01f89a Mon Sep 17 00:00:00 2001 From: Nikita Kodenko Date: Tue, 2 Jun 2020 22:53:08 +0700 Subject: [PATCH 3/6] run: simplified metrics check --- dvc/output/base.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/dvc/output/base.py b/dvc/output/base.py index 9bbf0aafae..bc0ace54e1 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -246,19 +246,17 @@ def save(self): self.ignore() + if self.metric or self.plot: + self.verify_metric() + if not self.use_cache: self.info = self.save_info() - if self.metric or self.plot: - self.verify_metric() if not self.IS_DEPENDENCY: logger.debug( "Output '%s' doesn't use cache. Skipping saving.", self ) return - if self.metric: - self.verify_metric() - assert not self.IS_DEPENDENCY if not self.changed(): From 1061ed598d75fd4fd9310944daf420c6f84449e8 Mon Sep 17 00:00:00 2001 From: Nikita Kodenko Date: Wed, 3 Jun 2020 23:14:10 +0700 Subject: [PATCH 4/6] tests: func: run: dirs as metrics outs --- dvc/output/local.py | 2 +- tests/func/test_run_single_stage.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/dvc/output/local.py b/dvc/output/local.py index f84d3478b4..f7d153efe8 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -81,7 +81,7 @@ def verify_metric(self): name = "metrics" if self.metric else "plot" if os.path.isdir(path): msg = "directory '{}' cannot be used as {}." - raise DvcException(msg.format(self.path_info, name)) + raise IsADirectoryError(msg.format(self.path_info, name)) if not istextfile(path): msg = "binary file '{}' cannot be used as {}." diff --git a/tests/func/test_run_single_stage.py b/tests/func/test_run_single_stage.py index 57f0a9d2c0..c5c080dec8 100644 --- a/tests/func/test_run_single_stage.py +++ b/tests/func/test_run_single_stage.py @@ -989,3 +989,32 @@ def test_should_raise_on_stage_output(tmp_dir, dvc, run_copy): with pytest.raises(OutputIsStageFileError): run_copy("foo", "name.dvc", single_stage=True) + + +class TestRunDirMetrics(TestDvc): + def setUp(self): + super().setUp() + with open(self.CODE, "w+") as fobj: + fobj.write("import sys\n") + fobj.write("import os\n") + fobj.write("os.makedirs(sys.argv[1])\n") + fobj.write( + "with open(os.path.join(sys.argv[1], 'metrics.json'), 'a+') as fobj:\n" + ) + fobj.write(" fobj.write('foo')\n") + + def test_metrics_dir_cached(self): + with self.assertRaises(IsADirectoryError): + self.dvc.run( + cmd=f"python {self.CODE} {self.DATA_DIR}", + metrics=[self.DATA_DIR], + single_stage=True, + ) + + def test_metrics_dir_not_cached(self): + with self.assertRaises(IsADirectoryError): + self.dvc.run( + cmd=f"python {self.CODE} {self.DATA_DIR}", + metrics_no_cache=[self.DATA_DIR], + single_stage=True, + ) From e50c58ca9a77efd3f78bccb62d0ea34483568f65 Mon Sep 17 00:00:00 2001 From: Nikita Kodenko Date: Wed, 3 Jun 2020 23:30:36 +0700 Subject: [PATCH 5/6] tests: func: run: reformatted string literals --- tests/func/test_run_single_stage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/func/test_run_single_stage.py b/tests/func/test_run_single_stage.py index c5c080dec8..5fd728cbbf 100644 --- a/tests/func/test_run_single_stage.py +++ b/tests/func/test_run_single_stage.py @@ -999,7 +999,8 @@ def setUp(self): fobj.write("import os\n") fobj.write("os.makedirs(sys.argv[1])\n") fobj.write( - "with open(os.path.join(sys.argv[1], 'metrics.json'), 'a+') as fobj:\n" + "with open(os.path.join(sys.argv[1], " + "'metrics.json'), 'a+') as fobj:\n" ) fobj.write(" fobj.write('foo')\n") From 1b2d5cb9ad3245119b3bc88914f9ed8d68c3ec99 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 4 Jun 2020 07:36:03 +0300 Subject: [PATCH 6/6] tests: use pytest --- tests/func/test_run_single_stage.py | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/tests/func/test_run_single_stage.py b/tests/func/test_run_single_stage.py index 5fd728cbbf..b92b9bc82e 100644 --- a/tests/func/test_run_single_stage.py +++ b/tests/func/test_run_single_stage.py @@ -991,10 +991,10 @@ def test_should_raise_on_stage_output(tmp_dir, dvc, run_copy): run_copy("foo", "name.dvc", single_stage=True) -class TestRunDirMetrics(TestDvc): - def setUp(self): - super().setUp() - with open(self.CODE, "w+") as fobj: +class TestRunDirMetrics: + @pytest.fixture(autouse=True) + def setup(self, dvc): + with open("script.py", "w+") as fobj: fobj.write("import sys\n") fobj.write("import os\n") fobj.write("os.makedirs(sys.argv[1])\n") @@ -1004,18 +1004,16 @@ def setUp(self): ) fobj.write(" fobj.write('foo')\n") - def test_metrics_dir_cached(self): - with self.assertRaises(IsADirectoryError): - self.dvc.run( - cmd=f"python {self.CODE} {self.DATA_DIR}", - metrics=[self.DATA_DIR], - single_stage=True, + def test_metrics_dir_cached(self, dvc): + with pytest.raises(IsADirectoryError): + dvc.run( + cmd="python script.py dir", metrics=["dir"], single_stage=True, ) - def test_metrics_dir_not_cached(self): - with self.assertRaises(IsADirectoryError): - self.dvc.run( - cmd=f"python {self.CODE} {self.DATA_DIR}", - metrics_no_cache=[self.DATA_DIR], + def test_metrics_dir_not_cached(self, dvc): + with pytest.raises(IsADirectoryError): + dvc.run( + cmd="python script.py dir", + metrics_no_cache=["dir"], single_stage=True, )