-
Notifications
You must be signed in to change notification settings - Fork 5
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
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #129 +/- ##
==========================================
+ Coverage 94.49% 95.51% +1.02%
==========================================
Files 19 19
Lines 726 758 +32
Branches 76 120 +44
==========================================
+ Hits 686 724 +38
+ Misses 27 19 -8
- Partials 13 15 +2
☔ View full report in Codecov by Sentry. |
@@ -120,5 +121,8 @@ def generate_markdown(self, report_path=None) -> str: | |||
plt.savefig(output_file) | |||
plt.close() | |||
|
|||
return f"\n![{self.name}]({output_file.relative_to(report_folder)})" |
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.
I think this actually breaks the integration with CML like:
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?
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.
Updated to make the file optional and to return the markdown string if no file is passed. PTAL.
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.
Updating dvclive to use this option. Should work the same as before as long as a file is passed.
393fcf8
to
510d294
Compare
510d294
to
e9fc84c
Compare
return output_file | ||
|
||
return document.embed() |
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.
Like this change, makes also sense for HTML
To support markdown reports in notebooks without needing to source from external files