plots: standardise across DVC/Studio/VS Code

[mattseddon]

• ❗ 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. 🙏

---

Related to #9940

This PR contains changes to accept the version of dvc-render created by iterative/dvc-render#142. See that PR for a detailed description of the overall change.

Docs:

• plots: standardise across DVC/Studio/VS Code dvc.org#4978

Downstream:

• Bump min DVC version to 3.33.0 (standardise plots across products) vscode-dvc#4734
• https://github.com/iterative/studio/pull/8242

[mattseddon]

[F] The reason for having this inside of a nested dict was because we wanted to get rid of the data before it showed up in a tooltip. However, none of the tooltips from the updated templates will show any of this data. It is therefore safe to remove.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1aed230) 90.35% compared to head (984860b) 90.35%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9931   +/-   ##
=======================================
  Coverage   90.35%   90.35%           
=======================================
  Files         496      496           
  Lines       37754    37737   -17     
  Branches     5490     5485    -5     
=======================================
- Hits        34113    34099   -14     
+ Misses       2980     2978    -2     
+ Partials      661      660    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dberenbaum]

What I can recall from #9019 and iterative/dvc-render#119 is:

1. We need to keep the rev field as is (it is actually not just the revision but actually like rev::file::field) to not break for custom templates that depend on it.
2. Having a combined field like rev::file::field as a unique ID for a line/series is useful for things like yOffset in vega-lite.
3. Not directly related/needed, but Use dvc_... fields from https://github.com/iterative/dvc/pull/9019 dvc-render#119 also includes consolidation of the strokeDash and shape so they don't need to be configured in downstream products.

[mattseddon]

What I can recall from #9019 and iterative/dvc-render#119 is:

1. We need to keep the rev field as is (it is actually not just the revision but actually like rev::file::field) to not break for custom templates that depend on it.
2. Having a combined field like rev::file::field as a unique ID for a line/series is useful for things like yOffset in vega-lite.
3. Not directly related/needed, but Use dvc_... fields from https://github.com/iterative/dvc/pull/9019 dvc-render#119 also includes consolidation of the strokeDash and shape so they don't need to be configured in downstream products.

Thanks @dberenbaum. All good information.

For 1. IIUC this only happened for the HTML output and not in Studio/VS Code. Is that correct?

[dberenbaum]

For 1. IIUC this only happened for the HTML output and not in Studio/VS Code. Is that correct?

Not sure I follow. If I use a custom template and that template includes the field rev, wouldn't it break even in VS Code and Studio if we change what's in that field or drop that field? You know better than me how we handle custom templates in VS Code and Studio.

[dberenbaum]

@mattseddon Where did we land on breaking changes for custom templates? Are there known use cases that will or won't work with old custom templates?

[dberenbaum]

@mattseddon This one looks nice! Good to see that all that added code in dvc-render at least made dvc a bit cleaner 😄 (and I assume made VS Code and Studio much cleaner). Once dvc-render is released and we update the pinned version here, I'll approve.

[mattseddon]

@mattseddon Where did we land on breaking changes for custom templates? Are there known use cases that will or won't work with old custom templates?

I feel like we've verbally agreed on this a few times but there is at least one reference in #9940 here: #9940 (comment)
and we have this in the assumptions section of the description:

Usage of custom templates is low enough not to worry about being backwards compatible with user-defined custom templates.

With that said custom templates should not completely break, but they may not render as expected because we will no longer be injecting values into them with VS Code or Studio.

[dberenbaum]

I feel like we've verbally agreed on this a few times but there is at least one reference in #9940 here: #9940 (comment) and we have this in the assumptions section of the description:

Usage of custom templates is low enough not to worry about being backwards compatible with user-defined custom templates.

We have definitely agreed before. I just couldn't remember exactly what we agreed, so thanks for the refresher. Do you think there's anything helpful to document about migrating old custom templates?

[dberenbaum]

@mattseddon I'm approving to unblock you, but please make sure not to forget the version bump.