-
Notifications
You must be signed in to change notification settings - Fork 213
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 _repr_mimebundle_ for IPython.display #150
Conversation
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.
Thanks for the PR.
Implementation looks good in general. :)
- I think
_repr_mimebundle_()
does the right thing. Proposed some simplifications - We will need to move the code in place a bit and make it more consistent with other parts, see my suggestions. Hope this helps.
- Can you add type annotations?
Then there is the whole side of testing and documentation: It would be great if you can try to take a look at the tests and the documentation (see also https://graphviz.readthedocs.io/en/latest/development.html) and at least make sure no tests are broken and there is at least some code coverage of the added (non-module-level) code.But if you prefer me to take over, that is fine as well. Feel free to propose documentation and changelog entry. Maybe it would make sense to add a docstring and include the method in the API docs (treat it as pseudo-private), WDYT?
Please check flake8 in tests result (when they pass again).
I will check the I have a question regarding the parameters folder. Should I create a new file in Btw. when you think it takes less time for you, when you take over instead of suggesting changes, feel free to do so.
Sure.
Sure, I try to get all test working, including the style checks. |
All good. I am fine with making suggestions for now (can take over when one/both of us think it's time).
I would say no: this submodule is stricly for parameters for rendering:
|
I tried now to be as close as possible to the code you mentioned. In |
Thanks. Can you squash the PR into one commit? If you are fine with me moving things, adapting naming and possibly more things, I can take over (if you have enabled it, I can add another commit to the PR branch so we can keep things separate after merging). SG? pytype:
|
7cb8675
to
06d0b2d
Compare
Yes, I squashed now all changes.
Yes, you can take over. I fixed the typing you mentioned and renamed |
…lake8 - also: add test coverage for include/exclude and fix behaviour
Codecov Report
@@ Coverage Diff @@
## master #150 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 32 32
Lines 860 891 +31
=========================================
+ Hits 860 891 +31
Continue to review full report at Codecov.
|
Bumps [graphviz](https://github.com/xflr6/graphviz) from 0.18.2 to 0.19. <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/xflr6/graphviz/blob/master/CHANGES.rst">graphviz's changelog</a>.</em></p> <blockquote> <h2>Version 0.19</h2> <p>Add <code>PendingDeprecationWarning</code> to calls using positional arguments that will be <strong>deprecated in a later version</strong>. The future API will allow from one to three positional arguments depending on the method or function. Keyword-only arguments where not around when this library was created. This signals dependents and in general users to start updating or pinning to the wanted version (or range). Crucially, this helps new users with a safer API that allows to avoid some common mistakes. Warnings reported in tests.</p> <p>Add keyword-only <code>outfile</code> argument to <code>.render()</code> and stand-alone <code>graphviz.render()</code>. Allows to override the rendered output file name: <code>.render(filename='spam.gv', outfile='spam.pdf')</code> Allows to derive the <code>format</code> and the <code>filename</code> from the rendered <code>outfile</code> name: <code>.render(outfile='spam.svg')</code> Tries to infer default <code>format</code> from the <code>outfile</code> suffix. You can override by setting <code>format</code> explicitly. Warns with a <code>graphviz.FormatSuffixMismatchWarning</code> if there is a mismatch between given <code>format</code> and the inferred format from <code>outfile</code> suffix. Warns with a <code>graphviz.UnknownSuffixWarning</code> if <code>format</code> is given and <code>outfile</code> uses a suffix that cannot be mapped to a supported format.</p> <p>Add <code>graphviz.set_jupyter_format()</code> to set the output <code>format</code> used by the Jupyter visualization of <code>graphviz.Graph</code>, <code>graphviz.Digraph</code>, and <code>graphviz.Source</code> (supported formats: <code>'svg'</code>, <code>'png'</code>, <code>'jpeg'</code>). Replace <code>_repr_svg_()</code> internally with <code>_repr_mimebundle_(include, exclude)</code> returning a mimebundle <code>{'image/svg+xml', '<?xml version=...'}</code> by default. Adds support for <code>IPython.display.display_png()</code>. Adds support for <code>IPython.display.display_jpeg()</code>. PR <code>[#150](xflr6/graphviz#150) <https://github.com/xflr6/graphviz/pull/150></code>_ Christoph Boeddeker.</p> <p>Add keyword-only <code>raise_if_result_exists</code> argument to <code>.render()</code> and stand-alone <code>graphviz.render()</code>. Raises <code>graphviz.FileExistsError</code> if the rendered file already exists.</p> <p>Add support to for <code>.render()</code> and stand-alone <code>.render()</code> to overwrite the input source file with the rendered output when using the <code>outfile</code> keyword-only argument. This probably only makes sense for text-based Graphviz formats such as <code>dot</code> or <code>plain</code>. You need to specify <code>overwrite_filepath=True</code> to enable this.</p> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/xflr6/graphviz/commit/e3d93611046c79efad212e62f039cd261b0a16f7"><code>e3d9361</code></a> release 0.19</li> <li><a href="https://github.com/xflr6/graphviz/commit/702b0c24324469df259e9d9cbf6e6d37b95f7f9d"><code>702b0c2</code></a> fix PyPI rendering error (?)</li> <li><a href="https://github.com/xflr6/graphviz/commit/01a3f00bb5351de39c535a5c71ebb50a55840cbd"><code>01a3f00</code></a> fix markup</li> <li><a href="https://github.com/xflr6/graphviz/commit/e2185cd7e3f5a644c533a9ee7f603c9375abc7f1"><code>e2185cd</code></a> reorder steps</li> <li><a href="https://github.com/xflr6/graphviz/commit/27a3a2afd6a76f57d0a52bca578daac8adb267fc"><code>27a3a2a</code></a> fix typo</li> <li><a href="https://github.com/xflr6/graphviz/commit/abced02611a72bccb8dccee07fd0475e576e506d"><code>abced02</code></a> move warning on empty file to expected</li> <li><a href="https://github.com/xflr6/graphviz/commit/99b4e628ad7407b6e7bb01fb9003956a1a296006"><code>99b4e62</code></a> add pytest.deprecated_call to expected deprecation warnings in tests</li> <li><a href="https://github.com/xflr6/graphviz/commit/13ab1d537ebf6c76f79701dcab75ba9ef2771b0c"><code>13ab1d5</code></a> fix internal deprecation warnings</li> <li><a href="https://github.com/xflr6/graphviz/commit/683c9c3e2bb6d415bca7d780062de79a831fd979"><code>683c9c3</code></a> flake8</li> <li><a href="https://github.com/xflr6/graphviz/commit/61ec0d8c876cc530ae8e8192c393ca844e3345a3"><code>61ec0d8</code></a> polish update-help.py</li> <li>Additional commits viewable in <a href="https://github.com/xflr6/graphviz/compare/0.18.2...0.19">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=graphviz&package-manager=pip&previous-version=0.18.2&new-version=0.19)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details>
Bumps [graphviz](https://github.com/xflr6/graphviz) from 0.18.2 to 0.19. <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/xflr6/graphviz/blob/master/CHANGES.rst">graphviz's changelog</a>.</em></p> <blockquote> <h2>Version 0.19</h2> <p>Add <code>PendingDeprecationWarning</code> to calls using positional arguments that will be <strong>deprecated in a later version</strong>. The future API will allow from one to three positional arguments depending on the method or function. Keyword-only arguments where not around when this library was created. This signals dependents and in general users to start updating or pinning to the wanted version (or range). Crucially, this helps new users with a safer API that allows to avoid some common mistakes. Warnings reported in tests.</p> <p>Add keyword-only <code>outfile</code> argument to <code>.render()</code> and stand-alone <code>graphviz.render()</code>. Allows to override the rendered output file name: <code>.render(filename='spam.gv', outfile='spam.pdf')</code> Allows to derive the <code>format</code> and the <code>filename</code> from the rendered <code>outfile</code> name: <code>.render(outfile='spam.svg')</code> Tries to infer default <code>format</code> from the <code>outfile</code> suffix. You can override by setting <code>format</code> explicitly. Warns with a <code>graphviz.FormatSuffixMismatchWarning</code> if there is a mismatch between given <code>format</code> and the inferred format from <code>outfile</code> suffix. Warns with a <code>graphviz.UnknownSuffixWarning</code> if <code>format</code> is given and <code>outfile</code> uses a suffix that cannot be mapped to a supported format.</p> <p>Add <code>graphviz.set_jupyter_format()</code> to set the output <code>format</code> used by the Jupyter visualization of <code>graphviz.Graph</code>, <code>graphviz.Digraph</code>, and <code>graphviz.Source</code> (supported formats: <code>'svg'</code>, <code>'png'</code>, <code>'jpeg'</code>). Replace <code>_repr_svg_()</code> internally with <code>_repr_mimebundle_(include, exclude)</code> returning a mimebundle <code>{'image/svg+xml', '<?xml version=...'}</code> by default. Adds support for <code>IPython.display.display_png()</code>. Adds support for <code>IPython.display.display_jpeg()</code>. PR <code>[#150](xflr6/graphviz#150) <https://github.com/xflr6/graphviz/pull/150></code>_ Christoph Boeddeker.</p> <p>Add keyword-only <code>raise_if_result_exists</code> argument to <code>.render()</code> and stand-alone <code>graphviz.render()</code>. Raises <code>graphviz.FileExistsError</code> if the rendered file already exists.</p> <p>Add support to for <code>.render()</code> and stand-alone <code>.render()</code> to overwrite the input source file with the rendered output when using the <code>outfile</code> keyword-only argument. This probably only makes sense for text-based Graphviz formats such as <code>dot</code> or <code>plain</code>. You need to specify <code>overwrite_filepath=True</code> to enable this.</p> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/xflr6/graphviz/commit/e3d93611046c79efad212e62f039cd261b0a16f7"><code>e3d9361</code></a> release 0.19</li> <li><a href="https://github.com/xflr6/graphviz/commit/702b0c24324469df259e9d9cbf6e6d37b95f7f9d"><code>702b0c2</code></a> fix PyPI rendering error (?)</li> <li><a href="https://github.com/xflr6/graphviz/commit/01a3f00bb5351de39c535a5c71ebb50a55840cbd"><code>01a3f00</code></a> fix markup</li> <li><a href="https://github.com/xflr6/graphviz/commit/e2185cd7e3f5a644c533a9ee7f603c9375abc7f1"><code>e2185cd</code></a> reorder steps</li> <li><a href="https://github.com/xflr6/graphviz/commit/27a3a2afd6a76f57d0a52bca578daac8adb267fc"><code>27a3a2a</code></a> fix typo</li> <li><a href="https://github.com/xflr6/graphviz/commit/abced02611a72bccb8dccee07fd0475e576e506d"><code>abced02</code></a> move warning on empty file to expected</li> <li><a href="https://github.com/xflr6/graphviz/commit/99b4e628ad7407b6e7bb01fb9003956a1a296006"><code>99b4e62</code></a> add pytest.deprecated_call to expected deprecation warnings in tests</li> <li><a href="https://github.com/xflr6/graphviz/commit/13ab1d537ebf6c76f79701dcab75ba9ef2771b0c"><code>13ab1d5</code></a> fix internal deprecation warnings</li> <li><a href="https://github.com/xflr6/graphviz/commit/683c9c3e2bb6d415bca7d780062de79a831fd979"><code>683c9c3</code></a> flake8</li> <li><a href="https://github.com/xflr6/graphviz/commit/61ec0d8c876cc530ae8e8192c393ca844e3345a3"><code>61ec0d8</code></a> polish update-help.py</li> <li>Additional commits viewable in <a href="https://github.com/xflr6/graphviz/compare/0.18.2...0.19">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=graphviz&package-manager=pip&previous-version=0.18.2&new-version=0.19)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details>
Address #138
@xflr6 Thank you for the modifications of the graphviz code base.
I followed your suggestion and implemented the
_repr_mimebundle_
.For SVG, PNG and JPEG it works, but PDF and JSON didn't work.
In the IPython documentation is not much info, so I was not able to figure out, why they don't work.
(For PDF, they may expect a file)
I didn't change the name for
JupyterSvgIntegration
, because I wanted to wait for feedback.