run/metrics: bad file format validation and error handling [qa]

[jorgeorpinel]

Bug Report

I get a strange error when trying to use dvc metrics show on a plaintext metrics file:

λ dvc run -n metricsnc -M metric.txt "echo metric > metric.txt"
Running callback stage 'metricsnc' with command:
        echo metric > metric.txt
Adding stage 'metricsnc' in 'dvc.yaml'
Updating lock file 'dvc.lock'
...
λ cat dvc.yaml
...
  metricsnc:
    cmd: echo metric > metric.txt
    metrics:
    - metric.txt:
        cache: false
λ dvc metrics show
ERROR: failed to show metrics - no metric files in this repository. Use `-m/-M` options for `dvc run` to mark stage outputs as  metrics.

Questions:

• Should dvc run try to check metrics/plots file formats and at least WARN about non-supported mime/types?
• Why is metrics show not even finding the metrics file, which is shown in the dvc.yaml file?

UPDATE: See #4068 (comment)

Please provide information about your setup

Output of dvc version:

λ dvc version
DVC version: 1.0.0b5+766ef3
Python version: 3.8.2
Platform: Windows-10-10.0.18362-SP0
Binary: False
Package: None
Supported remotes: azure, gdrive, gs, hdfs, http, https, s3, ssh, oss
Cache: reflink - not supported, hardlink - supported, symlink - supported
Filesystem type (cache directory): ('NTFS', 'C:\\')
Repo: dvc, git
Filesystem type (workspace): ('NTFS', 'C:\\')

Additional Information (if any):

If applicable, please also provide a --verbose output of the command, eg: dvc add --verbose.

[efiop]

Should dvc run try to check metrics/plots file formats and at least WARN about non-supported mime/types?

Actually, this is a very good idea! Let's do that!

Why is metrics show not even finding the metrics file, which is shown in the dvc.yaml file?

We've made a conscious decision to just ignore bad formats, because the warnings would get unwieldy very fast. Mind that we've dropped support for csv/tsv/etc metrics, which means that history in some projects might be really bad.

I really like your idea with dvc run, we definitely should look into it!

[jorgeorpinel]

We've made a conscious decision to just ignore bad formats, the warnings would get unwieldy

Makes sense but the error is confusing/ feels like a bug because I'm seeing the metrics file in dvc.yaml

Even if the type validation at dvc run -m/M gets implemented, would it also be a good idea to introduce a different error for this particular case? E.g. for those older projects with no-longer-supported metrics formats. Up to you 🙂

Thanks!

[nik123]

dvc metrics should probably also handle empty metrics files. For example in my training stage I created metrics file but accidentally haven't write anything into it. So I have metrics file in supported format but metrics show still shows "no metric files" error. I think there should be another message for such cases.

[efiop]

@nik123 Good point! We've merged #4370 yesterday, that provides a more descriptive error if it wasn't able to parse-out metrics. Could you give it a try and let us know if that fixes the issue for you?

[nik123]

@efiop It seems my issue still remains (dvc 1.5.1+147450). I still see "no metric files in repository..." message. I guess it's because my file was absolutely empty.