plots: load templates from package

[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. 🙏

Fixes #6513
Prerequisit to #6389

This change makes the default plot interactive. The previous default plot is now named "simple".
Since we are changing the default template, the repositories created before this change would not be affected by this change. In order to prevent that we remove dumping plots templates on dvc init and make dvc try to match template by name (same as we do in the case of non-dvc repositories).

One drawback is that users lose the ability to edit default templates, as well as producing their own templates is harder since default ones are not simply under .dvc/plots. We could fix that by improving our docs with the "Create your custom template" example in plots documentation.

Marking as draft to not merge before appropriate docs update.

EDIT:

This change is moving the templates from within the code to package data. The order of looking for template when provided in case of dvc plots show/diff is cwd -> .dvc/plots -> package_data.

After this change the default plot will be linear, and previous default is renamed to simple.

In following change I will introduce command letting user dump the templates into .dvc/plots

[dberenbaum]

The previous default plot is now named "simple".

I was thinking that these two templates should be called "linear" and "simple" or "linear_simple" rather than calling either "default." I think it's more helpful to have descriptive names even for the default, and it's easy to document that the default is "linear." In fact, the current documentation is confusing in that it doesn't differentiate between "default" and "linear" even though they are different templates: https://dvc.org/doc/command-reference/plots#plot-templates. Thoughts?

---

One drawback is that users lose the ability to edit default templates, as well as producing their own templates is harder since default ones are not simply under .dvc/plots. We could fix that by improving our docs with the "Create your custom template" example in plots documentation.

I'm not sure what's best, or whether this should be included in this PR.

• Is it expected that existing plots may change when updating DVC versions?
• What happens if someone adds a "smooth.json" with a different format?

One drawback is that users lose the ability to edit default templates, as well as producing their own templates is harder since default ones are not simply under .dvc/plots. We could fix that by improving our docs with the "Create your custom template" example in plots documentation.

A couple other ideas that might help with this problem:

• Include only the simple linear json file and treat this as an example custom plot (get rid of the existing simple linear template class).
• Read the content for the built-in templates from json files inside the package as package data. Is there a benefit to keeping the full template content as a massive dict inside each class definition?

[pared]

I was thinking that these two templates should be called "linear" and "simple" or "linear_simple" rather than calling either "default."

That makes more sense, I will do proper changes.

Is it expected that existing plots may change when updating DVC versions?

This was never discussed, I think the main reason to dump the templates was to make them easily accessible for users so that they can modify default ones, and start working on their own just by copying pre-existing ones.

What happens if someone adds a "smooth.json" with a different format?

Depends where. If it will be put into .dvc/plots - it will overwrite existing template and user will be able to access it by using --template smooth. If it will be somewhere else in repo, user needs to specify full path and filename. (using smooth without .json was a convenience feature added to make access to plots stored in .dvc/plots easy).

Include only the simple linear json file and treat this as an example custom plot (get rid of the existing simple linear template class).

Do I understand right? Include only simple.json in .dvc/plots?

Read the content for the built-in templates from json files inside the package as package data. Is there a benefit to keeping the full template content as a massive dict inside each class definition?

Makes sense, it would tie the plot to DVC version, and not the repository.

I think that one more reason to get rid of the templates from repo is that it creates additional files that some users consider garbage. #6389
We could also make plots lazier - initialize them in repo only when they are used the first time.

[dberenbaum]

cc @skshetry

[dberenbaum]

Do I understand right? Include only simple.json in .dvc/plots?

👍 That's my suggestion.

To my understanding, current issues are:

• Templates may be out of date relative to dvc version.
• Including in .dvc/plots makes it easier for users to understand and create custom templates.
• Including the templates creates a lot of unnecessary additional files for many users.

So I propose:

• Include only simple.json (or maybe call it example.json) in .dvc/plots (and get rid of the Python class for this template altogether).
• Read the contents of the other included templates directly from the json files (so that we can still link to those for docs and users can copy them), but bundle those files with the package instead of the repo.

By the way, can templates be in $HOME/.config/dvc/plots for reuse across projects?

Interested to hear what you and others think.

@skshetry Do you think this makes sense for stage templates in dvc exp init?

@shcheklein Do you see any impacts for other products?

[shcheklein]

It might affect a bit other products. It should be fine with Studio (since it's Python), it might not be as fine for VS Code. We might need an interface to access templates in some way (e.g. to determine if that template merges data or not). I think it's doable even if dvc returns fully baked Vega JS in such cases, but it might be less efficient.

I think the point here is that by hiding them in the package we are indeed increasing complexity of people accessing them (manually, programmatically), so I would weight the benefits one more time to be honest. Not an obvious thing to do.

[dberenbaum]

@pared Even though it will not get automatically pushed to existing repos, can we merge the changes to those linear templates for now and discuss the template organization in a separate issue/PR?

[pared]

@dberenbaum I added update to issue description.

[pared]

Preserving previous behaviour of writing default templates to .dvc/plots. It will be removed in next change.

[dberenbaum]

Thanks @pared! I meant that this PR could be limited to making the linear plot the default and renaming the current default. I didn't expect you to already move all the templates into json files in a pkg resources dir, but not sure it's worth reverting at this point.

[pared]

@pmrowla, @dberenbaum, the PR is ready for review

[dberenbaum]

Is this temporary for backwards compatibility or is intended that we will keep it long-term?

Plots included in pkg dir + being able to specify custom template path should be enough, and dropping TEMPLATES_DIR will ignore outdated templates in .dvc/plots.

[pared]

The problem here is what in a case when user modified hers/his plots in .dvc/plots? If we drop .dvc/plots user will need to specify the path. It's not a big problem but will be annoying. Also .dvc/plots seems like a good default place for users to put their own templates if they don't want to clutter their repo with templates dir. On the other hand, repos created pre-this-release will keep using outdated templates which is also not good.

We can always consider dropping .dvc/plots in 3.0

[dberenbaum]

Let's document how users with old repos can get updated templates by deleting the old ones. Also, are you planning a follow-up to dump the templates?

[pared]

Yes, I didn't include it here to not clutter it anymore.

[dberenbaum]

It might be good to rename the PR since it seems to extend pretty far beyond the original title.

[pmrowla]

The changes on the code side LGTM

[skshetry]

This Traversable thing with files() is giving me ideas to implement this for dvc.api for directories. 😄

[skshetry]

@pared, you may also need to add files to the pyinstaller: https://pyinstaller.readthedocs.io/en/stable/spec-files.html#adding-files-to-the-bundle

[pared]

@skshetry thanks, can confirm it's not working for the binary package, preparing a fix...