metrics: accept any viable target in DvcRepo

[pared]

• ❗ I have followed the Contributing to DVC checklist.

• 📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

[pared]

@jorgeorpinel Do you think its worth mentioning in docs that from this change on dvc metrics will work with any file if its provided via targets?

[efiop]

Will recursive work with unofficial metrics? I don't see a test for it and the code seems like it won't work.

[pared]

I am not sure how that would work, I mean, in that case we would need to try to show all files for particular directory, since they are not marked as metrics. So the question here is whether we want to support that.

[efiop]

@pared That seems like a natural approach. You have a test with official metrics dir in this PR, what changes if they are not declared as official metrics?

[pared]

On the other hand, it kinda makes sense to have special dir for metrics and check all of them out. Ill try to modify the change.

[efiop]

Also note that we try to parse metric files, so if something is not a metric we'll just ignore it and won't show it.

[efiop]

What if a target is a file and recursive==True? Will it be added to the result twice?

[efiop]

This will probably become obsoleted once you implement recursive for unofficial metrics above...

[efiop]

Missing a test for recursive directory with unofficial metrics.

[pared]

Since both official and unofficial metrics are gathered the same way now, I would argue that test_non_metric_and_recurisve_show already covers that. Anyhow, I modified it so that it checks both official and unofficial metrics.

[efiop]

Thank you! 🙏

[efiop]

Thanks!

[jorgeorpinel]

Do you think its worth mentioning in docs

Yes! Very much 🙂 — thanks for submitting a docs PR BTW.