-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
vscode: support flexible plots #8282
Conversation
b38514b
to
6f85c03
Compare
@pared This seems to break bar plots at the moment. The values in the HTML are missing {
"Feature": "data frame",
"Mean decrease in impurity": "0.10044235196789414",
"dvc_data_version_info": {
"field": "Feature",
"filename": "feature_importance.csv",
"revision": "exp-fe059"
} |
@dberenbaum yep, hence the WIP, but I am not sure yet if we want to merge it, as I see that vscode team can handle that support even with existing DVC. Let's wait how the vscode implementation progresses, I can fix the tests when needed. |
Ok, as I see in iterative/vscode-dvc#1757 (comment) we do rely on it. I will update this PR |
6f85c03
to
2186b28
Compare
7a11b87
to
17e43f4
Compare
c4cec6f
to
50caa0d
Compare
assert html_result["linear.json"]["data"]["values"] == _update_datapoints( | ||
linear_v1, | ||
{ | ||
"rev": "workspace", | ||
REVISION_FIELD: "workspace", | ||
VERSION_FIELD: { | ||
"revision": "workspace", | ||
"filename": "linear.json", | ||
"field": "y", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the test is for VSCode integration, it probably makes more sense to test against the json result, right?
json_result["linear.json"][0]["datapoints"]["workspace"] == _update_datapoints(
linear_v1,
{
VERSION_FIELD: {
"revision": "workspace",
"filename": "linear.json",
"field": "y",
},
},
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be sure we need to check both, added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments. If it works for VSCode, lgtm
4458780
to
8dbbaa6
Compare
8dbbaa6
to
16128ae
Compare
β 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. π
Change required for iterative/vscode-dvc#2403
Related iterative/vscode-dvc#1757