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

add base64 support for markdown #129

Merged
merged 3 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/dvc_render/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def generate_markdown(self, report_path=None) -> str:
for datapoint in self.datapoints:
src = datapoint[self.SRC_FIELD]
if src.startswith("data:image;base64"):
raise ValueError("`generate_markdown` doesn't support base64")
src = src.replace("data:image;base64", "data:image/png;base64")
content.append(f"\n![{datapoint[self.TITLE_FIELD]}]({src})")
if content:
return "\n".join(content)
Expand Down
15 changes: 10 additions & 5 deletions src/dvc_render/markdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,14 @@ def embed(self) -> str:

def render_markdown(
renderers: List["Renderer"],
output_file: "StrPath",
output_file: Optional["StrPath"] = None,
template_path: Optional["StrPath"] = None,
) -> "StrPath":
"User renderers to fill an Markdown template and write to path."
output_path = Path(output_file)
output_path.parent.mkdir(exist_ok=True)
output_path = None
if output_file:
output_path = Path(output_file)
output_path.parent.mkdir(exist_ok=True)

page = None
if template_path:
Expand All @@ -63,6 +65,9 @@ def render_markdown(
for renderer in renderers:
document.with_element(renderer.generate_markdown(report_path=output_path))

output_path.write_text(document.embed(), encoding="utf8")
if output_file and output_path:
daavoo marked this conversation as resolved.
Show resolved Hide resolved
output_path.write_text(document.embed(), encoding="utf8")

return output_file
return output_file

return document.embed()
Comment on lines +71 to +73
Copy link
Contributor

Choose a reason for hiding this comment

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

Like this change, makes also sense for HTML

26 changes: 20 additions & 6 deletions src/dvc_render/vega.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import base64
import io
import json
from pathlib import Path
from typing import Any, Dict, List, Optional, Union
Expand Down Expand Up @@ -102,23 +104,35 @@ def generate_markdown(self, report_path=None) -> str:

data = list_dict_to_dict_list(self.datapoints)
if data:
report_folder = Path(report_path).parent
output_file = report_folder / self.name
output_file = output_file.with_suffix(".png")
output_file.parent.mkdir(exist_ok=True, parents=True)
if report_path:
report_folder = Path(report_path).parent
output_file = report_folder / self.name
output_file = output_file.with_suffix(".png")
output_file.parent.mkdir(exist_ok=True, parents=True)
else:
output_file = io.BytesIO() # type: ignore

x = self.properties.get("x")
y = self.properties.get("y")
data[x] = list(map(float, data[x]))
data[y] = list(map(float, data[y]))

plt.title(self.properties.get("title", output_file.stem))
plt.title(self.properties.get("title", Path(self.name).stem))
plt.xlabel(self.properties.get("x_label", x))
plt.ylabel(self.properties.get("y_label", y))
plt.plot(x, y, data=data)
plt.tight_layout()
plt.savefig(output_file)
plt.close()

return f"\n![{self.name}]({output_file.relative_to(report_folder)})"
Copy link
Contributor

@daavoo daavoo May 10, 2023

Choose a reason for hiding this comment

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

I think this actually breaks the integration with CML like:

https://github.com/iterative/example-get-started-experiments/blob/main/.github/workflows/run-studio-experiment.yml#L61

Because there is some logic that resolves the image paths and uses CML upload under the hood.

Could we make the base64 optional, default to the previous behavior, and explicitly enable it on the DVCLive side only for notebooks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to make the file optional and to return the markdown string if no file is passed. PTAL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating dvclive to use this option. Should work the same as before as long as a file is passed.

if report_path:
return f"\n![{self.name}]({output_file.relative_to(report_folder)})"

base64_str = base64.b64encode(
output_file.getvalue() # type: ignore
).decode()
src = f"data:image/png;base64,{base64_str}"

return f"\n![{self.name}]({src})"

return ""
5 changes: 3 additions & 2 deletions tests/test_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ def test_invalid_generate_markdown():
"src": "data:image;base64,encoded_image",
}
]
with pytest.raises(ValueError, match="`generate_markdown` doesn't support base64"):
ImageRenderer(datapoints, "file.jpg").generate_markdown()
md = ImageRenderer(datapoints, "file.jpg").generate_markdown()

assert "![workspace](data:image/png;base64,encoded_image)" in md


@pytest.mark.parametrize(
Expand Down
18 changes: 16 additions & 2 deletions tests/test_markdown.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import pytest

from dvc_render.markdown import PAGE_MARKDOWN, Markdown, MissingPlaceholderError
from dvc_render.markdown import (
PAGE_MARKDOWN,
Markdown,
MissingPlaceholderError,
render_markdown,
)

# pylint: disable=missing-function-docstring, R0801

Expand All @@ -26,7 +31,7 @@
),
],
)
def test_html(template, page_elements, expected_page):
def test_markdown(template, page_elements, expected_page):
page = Markdown(template)
page.elements = page_elements

Expand All @@ -40,3 +45,12 @@ def test_no_placeholder():

with pytest.raises(MissingPlaceholderError):
Markdown(template)


def test_render_markdown_to_file(tmp_dir):
output_file = tmp_dir / "report"
assert output_file == render_markdown([], output_file)


def test_render_markdown_no_file():
assert "# DVC Report" in render_markdown([])
19 changes: 14 additions & 5 deletions tests/test_vega.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ def test_raise_on_wrong_field():


@pytest.mark.parametrize("name", ["foo", "foo/bar", "foo/bar.tsv"])
def test_generate_markdown(tmp_dir, mocker, name):
@pytest.mark.parametrize("to_file", [True, False])
def test_generate_markdown(tmp_dir, mocker, name, to_file):
# pylint: disable-msg=too-many-locals
import matplotlib.pyplot

plot = mocker.spy(matplotlib.pyplot, "plot")
Expand All @@ -131,10 +133,18 @@ def test_generate_markdown(tmp_dir, mocker, name):
]
renderer = VegaRenderer(datapoints, name, **props)

(tmp_dir / "output").mkdir()
renderer.generate_markdown(tmp_dir / "output" / "report.md")
if to_file:
report_folder = tmp_dir / "output"
report_folder.mkdir()
md = renderer.generate_markdown(tmp_dir / "output" / "report.md")
output_file = (tmp_dir / "output" / renderer.name).with_suffix(".png")
assert output_file.exists()
savefig.assert_called_with(output_file)
assert f"![{name}]({output_file.relative_to(report_folder)})" in md
else:
md = renderer.generate_markdown()
assert f"![{name}](data:image/png;base64," in md

assert (tmp_dir / "output" / renderer.name).with_suffix(".png").exists()
plot.assert_called_with(
"first_val",
"second_val",
Expand All @@ -147,7 +157,6 @@ def test_generate_markdown(tmp_dir, mocker, name):
title.assert_called_with("FOO")
xlabel.assert_called_with("first_val")
ylabel.assert_called_with("second_val")
savefig.assert_called_with((tmp_dir / "output" / name).with_suffix(".png"))


def test_unsupported_template():
Expand Down