Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plots: fail silently on errors #9441

Closed
dberenbaum opened this issue May 11, 2023 · 2 comments · Fixed by #9726
Closed

plots: fail silently on errors #9441

dberenbaum opened this issue May 11, 2023 · 2 comments · Fixed by #9726
Assignees
Labels
A: plots Related to the plots p1-important Important, aka current backlog of things to do regression Ohh, we broke something :-(

Comments

@dberenbaum
Copy link
Collaborator

dberenbaum commented May 11, 2023

Bug Report

Description

dvc plots show has started failing silently when there are errors instead of reporting an error was found. It looks like this was introduced in 43195a8 (cc @skshetry).

Reproduce

Clone https://github.com/iterative/vscode-dvc-demo and make these changes to dvc.yaml:

$ git diff
diff --git a/dvc.yaml b/dvc.yaml
index 3a4fa26..cd91913 100644
--- a/dvc.yaml
+++ b/dvc.yaml
@@ -2,8 +2,8 @@ plots:
   - Accuracy:
       x: step
       y:
-        training/plots/metrics/train/acc.tsv: acc
-        training/plots/metrics/test/acc.tsv: acc
+        training/plots/metrics/train/acc.tsv: train/acc
+        training/plots/metrics/test/acc.tsv: test/acc
       y_label: accuracy
   - Loss:
       x: step

Then run dvc plots show.

Expected

Prior to this change, dvc plots show would return an error message like this:

$ dvc plots show
ERROR: Could not find provided field ('train/acc') in data fields ('step, acc').
@dberenbaum dberenbaum added p1-important Important, aka current backlog of things to do regression Ohh, we broke something :-( A: plots Related to the plots labels May 11, 2023
@skshetry
Copy link
Member

I think I did it in this way to fix #9025.
Before we just used to fail, but now we reuse the same merged spec over multiple revisions data.

So, you may see multiple messages in this case if we logged them:

$ dvc plots diff workspace $(git log --oneline --pretty=format:"%h" -n2)
WARNING: Could not find provided field ('acc/loss') in data fields ('step, loss').                                                                                                                                   
WARNING: Could not find provided field ('acc/loss') in data fields ('step, loss').
WARNING: Could not find provided field ('acc/loss') in data fields ('step, loss').

@dberenbaum
Copy link
Collaborator Author

Makes sense for diff but what about for show?

@dberenbaum dberenbaum added this to DVC May 18, 2023
@dberenbaum dberenbaum moved this to Backlog in DVC May 18, 2023
@dberenbaum dberenbaum moved this from Backlog to Todo in DVC Jul 11, 2023
skshetry added a commit to skshetry/dvc that referenced this issue Jul 12, 2023
Fixes iterative#9441.

Note that we do merge props, and use that for all revision. So you may
see warning for multiple revisions.
@github-project-automation github-project-automation bot moved this from Todo to Done in DVC Jul 12, 2023
skshetry added a commit that referenced this issue Jul 12, 2023
Fixes #9441.

Note that we do merge props, and use that for all revision. So you may
see warning for multiple revisions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: plots Related to the plots p1-important Important, aka current backlog of things to do regression Ohh, we broke something :-(
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants