Skip to content

html: enable providing custom html page templates#5694

Merged
pared merged 2 commits into
treeverse:masterfrom
pared:custom_plots_html
Apr 20, 2021
Merged

html: enable providing custom html page templates#5694
pared merged 2 commits into
treeverse:masterfrom
pared:custom_plots_html

Conversation

@pared
Copy link
Copy Markdown
Contributor

@pared pared commented Mar 25, 2021 •
edited by jorgeorpinel
Loading

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

Initial implementation allowing custom HTML webpage template.
I decided just to allow users provide their own HTML, as long as there is {plot_divs} format string inside, so that we don't have to deal with additional args like --vega-path --vega-lite-path --vega-embed-path which would be necessary if we were to handle the template on our own, in dvc code.

How one can utilize new functionality :

#!/bin/bash

set -ex
rm -rf repo
mkdir repo
pushd repo

echo '[{"m":1},{"m":2},{"m":3}]' >> plot.json

echo '<html>
<head>
    <script type="text/javascript" src="VEGA_PATH"></script>
    <script type="text/javascript" src="VEGA_LITE_PATH"></script>
    <script type="text/javascript" src="VEGA_EMBED_PATH"></script>
</head>
<body>
	{plot_divs}
</body>
</html> ' >> template

wget https://cdn.jsdelivr.net/npm/vega@5.10.0 -O VEGA_PATH
wget https://cdn.jsdelivr.net/npm/vega-lite@4.8.1 -O VEGA_LITE_PATH
wget https://cdn.jsdelivr.net/npm/vega-embed@6.5.1 -O VEGA_EMBED_PATH

dvc plots show plot.json --html-template template

For sake of simplicity there is only --html-template option. However if we decide to go this way (allowing users to provide their own HTML template) I will prepare proper dvc config command, that would allow doing the setup only once per project.

cc @JIoJIaJIu @mnrozhkov @dberenbaum

@pared pared added enhancement Enhances DVC ui user interface / interaction needs-review labels Mar 25, 2021
dberenbaum
dberenbaum approved these changes Mar 26, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@dberenbaum dberenbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Another alternative would be to have a cdn-path or vega-base path or something that would replace the url used for all the vega scripts. We may end up revisiting the plots in the near future to address some of the other enhancements discussed, so whatever you and @JIoJIaJIu think is best for now.

Comment thread dvc/utils/html.py Outdated
Copy link
Copy Markdown
Contributor

@dberenbaum dberenbaum Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this print statement intended to stay in here?

@pared pared force-pushed the custom_plots_html branch from 17c5def to 7579738 Compare March 26, 2021 13:48
@pared pared force-pushed the custom_plots_html branch from 7579738 to 0f1103a Compare April 9, 2021 10:24
@pared pared force-pushed the custom_plots_html branch from 0f1103a to 22c5916 Compare April 12, 2021 13:47
@pared
Copy link
Copy Markdown
Contributor Author

pared commented Apr 12, 2021

Added config option for html_template, here is the sample script modified to use new option:

#!/bin/bash

set -ex
rm -rf repo
mkdir repo
pushd repo

git init --quiet
dvc init --quiet

git add -A
git commit -am "init"

path=$(pwd)
path=$path/.dvc

echo "<html>
<head>
    <script type=\"text/javascript\" src=\"$path/VEGA_PATH\"></script>
    <script type=\"text/javascript\" src=\"$path/VEGA_LITE_PATH\"></script>
    <script type=\"text/javascript\" src=\"$path/VEGA_EMBED_PATH\"></script>
</head>
<body>
	{plot_divs}
</body>
</html>" >> .dvc/template.html

dvc config plots.html_template template.html

wget https://cdn.jsdelivr.net/npm/vega@5.10.0 -O .dvc/VEGA_PATH
wget https://cdn.jsdelivr.net/npm/vega-lite@4.8.1 -O .dvc/VEGA_LITE_PATH
wget https://cdn.jsdelivr.net/npm/vega-embed@6.5.1 -O .dvc/VEGA_EMBED_PATH

mkdir sub
pushd sub
echo '[{"m":1},{"m":2},{"m":3}]' >> plot.json
dvc plots show plot.json

Please note the fact that in this case using full paths in HTML template is necessity, that's the only way to make sure custom template works regardless where do we call plots from.

@dberenbaum
Copy link
Copy Markdown
Contributor

dberenbaum commented Apr 19, 2021

Fixes #5679

gurobokum
gurobokum approved these changes Apr 19, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@gurobokum gurobokum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pared pared merged commit d87ace0 into treeverse:master Apr 20, 2021
@pared pared mentioned this pull request Apr 20, 2021
Closed
@efiop efiop mentioned this pull request Apr 20, 2021
Closed
jorgeorpinel
jorgeorpinel reviewed Jun 23, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post-merge discussion per treeverse/dvc.org#2402 (comment) 👇

Comment thread dvc/config_schema.py
# enabled by default. It's of no use, kept for backward compatibility.
Optional("parametrization", default=True): Bool
},
"plots": {"html_template": str},
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel Jun 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be just plots.html? To avoid overloading the term "template" (also associated with Vega customizations).

Comment thread dvc/command/plots.py
Comment on lines +248 to +249
parser.add_argument(
"--html-template",
Copy link
Copy Markdown
Contributor

@jorgeorpinel jorgeorpinel Jun 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, should it be just --html?

Comment thread dvc/utils/html.py
@jorgeorpinel jorgeorpinel mentioned this pull request Jun 23, 2021
Merged
This was referenced Jun 24, 2021
Closed
Closed
@pared pared deleted the custom_plots_html branch January 4, 2022 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mnrozhkov mnrozhkov Awaiting requested review from mnrozhkov

4 more reviewers

@iesahin iesahin iesahin left review comments

@jorgeorpinel jorgeorpinel jorgeorpinel left review comments

@gurobokum gurobokum gurobokum approved these changes

@dberenbaum dberenbaum dberenbaum approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement Enhances DVC ui user interface / interaction

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@pared @dberenbaum @iesahin @gurobokum @jorgeorpinel @efiop @skshetry