Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 30 additions & 25 deletions dvc/repo/metrics/show.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
from __future__ import unicode_literals

import errno
import os
import csv
import json
import logging

from jsonpath_ng.ext import parse

from dvc.exceptions import OutputNotFoundError, BadMetricError, NoMetricsError
from dvc.utils.compat import builtin_str, open, StringIO, csv_reader

NO_METRICS_FILE_AT_REFERENCE_WARNING = (
"Metrics file '{}' does not exist at reference: '{}'"
)

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -147,7 +152,7 @@ def _read_metric(fd, typ=None, xpath=None, rel_path=None, branch=None):
return None


def _collect_metrics(self, path, recursive, typ, xpath, branch):
def _collect_metrics(repo, path, recursive, typ, xpath, branch):
"""Gather all the metric outputs.

Args:
Expand All @@ -165,11 +170,11 @@ def _collect_metrics(self, path, recursive, typ, xpath, branch):
- typ:
- xpath:
"""
outs = [out for stage in self.stages() for out in stage.outs]
outs = [out for stage in repo.stages() for out in stage.outs]

if path:
try:
outs = self.find_outs_by_path(path, outs=outs, recursive=recursive)
outs = repo.find_outs_by_path(path, outs=outs, recursive=recursive)
except OutputNotFoundError:
logger.debug(
"stage file not for found for '{}' in branch '{}'".format(
Expand All @@ -195,16 +200,7 @@ def _collect_metrics(self, path, recursive, typ, xpath, branch):
return res


def _read_metrics_filesystem(path, typ, xpath, rel_path, branch):
if not os.path.exists(path):
return None
with open(path, "r") as fd:
return _read_metric(
fd, typ=typ, xpath=xpath, rel_path=rel_path, branch=branch
)


def _read_metrics(self, metrics, branch):
def _read_metrics(repo, metrics, branch):
"""Read the content of each metric file and format it.

Args:
Expand All @@ -226,22 +222,31 @@ def _read_metrics(self, metrics, branch):
if not typ:
typ = os.path.splitext(out.path.lower())[1].replace(".", "")
if out.use_cache:
metric = _read_metrics_filesystem(
self.cache.local.get(out.checksum),
typ=typ,
xpath=xpath,
rel_path=out.rel_path,
branch=branch,
)
open_fun = open
path = repo.cache.local.get(out.checksum)
Copy link
Contributor

Choose a reason for hiding this comment

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

But in this case Metrics file '{}' does not exist at branch: '{}' doesn't really fit, since path is a cache file for metric file 🙂

Copy link
Contributor Author

@pared pared Apr 29, 2019

Choose a reason for hiding this comment

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

Not exactly, we pass out.relpath to warning message, which is "common part" for both use cases. We always get relative path of our out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, right. Thanks!

else:
with self.tree.open(out.path) as fd:
open_fun = repo.tree.open
path = out.path
try:

with open_fun(path) as fd:
metric = _read_metric(
fd,
typ=typ,
xpath=xpath,
rel_path=out.rel_path,
branch=branch,
)
except IOError as e:
if e.errno == errno.ENOENT:
logger.warning(
NO_METRICS_FILE_AT_REFERENCE_WARNING.format(
out.rel_path, branch
)
)
metric = None
else:
raise

if not metric:
continue
Expand All @@ -252,7 +257,7 @@ def _read_metrics(self, metrics, branch):


def show(
self,
repo,
path=None,
typ=None,
xpath=None,
Expand All @@ -262,9 +267,9 @@ def show(
):
res = {}

for branch in self.brancher(all_branches=all_branches, all_tags=all_tags):
entries = _collect_metrics(self, path, recursive, typ, xpath, branch)
metrics = _read_metrics(self, entries, branch)
for branch in repo.brancher(all_branches=all_branches, all_tags=all_tags):
entries = _collect_metrics(repo, path, recursive, typ, xpath, branch)
metrics = _read_metrics(repo, entries, branch)
if metrics:
res[branch] = metrics

Expand Down
49 changes: 49 additions & 0 deletions tests/func/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
from dvc.repo import Repo as DvcRepo
from dvc.main import main
from dvc.exceptions import DvcException, BadMetricError, NoMetricsError
from dvc.repo.metrics.show import NO_METRICS_FILE_AT_REFERENCE_WARNING
from dvc.stage import Stage
from tests.basic_env import TestDvc


Expand Down Expand Up @@ -775,3 +777,50 @@ def _do_show(self, file_name, xpath):
self.assertSequenceEqual(ret[branch][file_name], [branch])
else:
self.assertSequenceEqual(ret[branch][file_name], branch)


class TestShouldDisplayMetricsEvenIfMetricIsMissing(object):
BRANCH_MISSING_METRIC = "missing_metric_branch"
METRIC_FILE = "metric"
METRIC_FILE_STAGE = METRIC_FILE + Stage.STAGE_FILE_SUFFIX

def _write_metric(self):
with open(self.METRIC_FILE, "w+") as fd:
fd.write("0.5")
fd.flush()

def _commit_metric(self, dvc, branch):
dvc.scm.add([self.METRIC_FILE_STAGE])
dvc.scm.commit("{} commit".format(branch))

def setUp(self, dvc):
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just __init__ ?

Copy link
Contributor Author

@pared pared Apr 29, 2019

Choose a reason for hiding this comment

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

Pytest by default does not collect classes that have __init__()
https://docs.pytest.org/en/latest/goodpractices.html#conventions-for-python-test-discovery

Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't know that, thanks for clarifying! 🙂

dvc.scm.branch(self.BRANCH_MISSING_METRIC)

self._write_metric()

ret = main(["run", "-m", self.METRIC_FILE])
assert 0 == ret

self._commit_metric(dvc, "master")

def test(self, dvc, caplog):
self.setUp(dvc)

dvc.scm.checkout(self.BRANCH_MISSING_METRIC)

self._write_metric()
ret = main(["run", "-M", self.METRIC_FILE])
assert 0 == ret

self._commit_metric(dvc, self.BRANCH_MISSING_METRIC)
os.remove(self.METRIC_FILE)

ret = main(["metrics", "show", "-a"])

assert (
NO_METRICS_FILE_AT_REFERENCE_WARNING.format(
self.METRIC_FILE, self.BRANCH_MISSING_METRIC
)
in caplog.text
)
assert 0 == ret