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.

Sign up for GitHub

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

Jump to bottom

create an entity relationship diagrams of the schemas using erdantic #148

Merged
merged 42 commits into from Jun 13, 2023
Merged

create an entity relationship diagrams of the schemas using erdantic #148

merged 42 commits into from Jun 13, 2023

Conversation

yves-renier
Copy link
Contributor

Hello,

I wanted to have ERD diagram for my models so I implemented this.
I added an option model-erdantic-figure to switch it off (it is currently on by default, but feel free to change that).
I also added the documentation for the option.

@mansenfranzen
Copy link
Owner

mansenfranzen commented Apr 28, 2023 •
edited

Hi @yves-renier,

thanks for your PR! You did a great job diving deeply into sphinx and autodoc_pydantic internals ;-).

I really like the idea of adding the possibility to show ERDs for pydantic models. I'm happy to support this PR.

So far, there remain some ToDos that we should address first:

  • autodoc_pydantic will need to add erdantic as an dependency in its pyproject.toml. Otherwise, the docs build process runs into an import error.

  • erdantic depends on pygraphviz and graphviz which will complicate the installation process of autodoc_pydantic.

    • pygraphviz does not offer wheels and requires a C/C++ compiler.
    • graphviz is not a python but external system dependency which can't be installed via pip.
    • Using conda here would simplify the process of setting up the doc build environment but I would rather prefer to keep it plain poetry/pip.

  • Hence, I suggest that erdantic should be an optional dependency. Accordingly, the ERD feature should be turned off by default.

  • If ERD is enabled, we should add a check to test against appropriate dependencies and provide a meaningful error message to inform users.

    • This will require a short documentation on how to properly set up autodoc_pydantic with erdantic, pygraphviz and graphviz.

  • Since this is a great feature, I would very much like to see a dedicated example in the example section.

  • Currently, there are no tests for the new feature. This should be added. I can assist here since this is rather complicated due to the nature of sphinx tests.

Please feel free to comment on my thoughts here. I'm happy to discuss them.

@mansenfranzen mansenfranzen added the enhancement New feature or request label Apr 28, 2023
@yves-renier
Copy link
Contributor Author

yves-renier commented May 1, 2023 •
edited

Hello, thanks for the support !
I just added erdantic as an optional dependency (pip install autodoc_pydantic[erd]) and turned off the ERD feature by default.
I will check and document the dependencies and try to setup the tests locally.

@mansenfranzen
Copy link
Owner

Thanks for continuing with the PR!

If you get stuck or need any help, please don't hesitate to reach out.

@yves-renier
Copy link
Contributor Author

About example, I already added one at /docs/build/html/users/configuration.html#show-erdantic-figure.
I will also enable the option for the complete example.

@yves-renier
Show warnings when erdantic is not installed and an erdantic figure i…
58bfc2d 
…s asked.

Add documentation to install with erd option
@yves-renier
Copy link
Contributor Author

yves-renier commented May 2, 2023 •
edited

About enabling the option for the complete example, I see it is presently an autopydantic_settings while I think the ERD option only makes sense for an autopydantic_model.
I will add a new section after the complete example with the ERD.

@yves-renier
Copy link
Contributor Author

yves-renier commented May 2, 2023 •
edited

I just added a simple test.
@mansenfranzen, I believe I finished implementing your TODO.
I also modified the github actions for tests to install erdantic and graphviz.

@mansenfranzen mansenfranzen mentioned this pull request May 2, 2023
Closed
@mansenfranzen
Copy link
Owner

@yves-renier The PR looks great - well done!

I can confirm that tests pass and documentation is rendered properly. I have a few thoughts remaining that prevent me from merging, yet:

  • I dislike erdantic's watermark at the bottom of the diagram. It distracts. I would rather see it being removed by erdantic or by autodoc_pydantic. The latter option is of course less desirable. I opened an issue here.
  • The resulting diagram can be improved if rendered as SVG instead of PNG. SVG is superior because it is a vector and additional information is shown upon hovering over attributes of pydantic models. sphinx.ext.graphviz already provides a configuration to switch to SVG, see here. We could make this the default for autodoc_pydantic.
  • While seeing the generated diagrams in action for the first time in the newly build docs, I think the diagrams can be visually very dominant. I think we can improve here to make them collapsable just like the JSON schema.

I'm curious to get to know your thoughts on these points.

@yves-renier
Copy link
Contributor Author

yves-renier commented May 4, 2023 •
edited

  • The watermark made the test harder as erdantic version is mentioned in it which we don't know in advance. I ignored that part of the output in the tests I implemented. Having a customizable label would also make those kind of tests much cleaner. I totally agree with you that the best would be an option from erdantic as I would not be comfortable either with removing it from autodoc_pydantic.
  • about SVG format for the diagram, I just tested and it seems to works. I modified docs/source/conf.py accordingly.
  • about making the diagram collapsible, I have no problem about that, I will make it the default and add an other option model-erdantic-figure-collapsed to disable it.

mansenfranzen
mansenfranzen requested changes May 9, 2023
View reviewed changes
Copy link
Owner

@mansenfranzen mansenfranzen left a comment

Choose a reason for hiding this comment

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

As I said before, you've submitted great PR! I really appreciate the work you've put into the project.

I've added multiple small comments/recommendations. I hope they are self-explanatory. If not, please reach out.

I think we still need to include a short note/hint in the documentation about installing erdantic if the feature is enabled.

sphinxcontrib/autodoc_pydantic/directives/autodocumenters.py Outdated Show resolved Hide resolved
sphinxcontrib/autodoc_pydantic/directives/autodocumenters.py Outdated Show resolved Hide resolved
sphinxcontrib/autodoc_pydantic/directives/templates.py Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
docs/source/users/configuration.rst Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
@mansenfranzen
Copy link
Owner

@yves-renier By the way, great work adding model-erdantic-figure-collapsed along with tests!

@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:32 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:32 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:33 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:46 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:46 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:47 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:47 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:53 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:53 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:53 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:54 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:59 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 20:59 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 21:00 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 21:00 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 21:08 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 21:08 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 21:08 — with GitHub Actions Inactive
@mansenfranzen mansenfranzen temporarily deployed to ci June 13, 2023 21:09 — with GitHub Actions Inactive
@mansenfranzen
Copy link
Owner

adding a link in the Documentation section of the Readme.md would not hurt.

Agreed - I will incorporate that in a separate release.

One last thing, I did not manage to run sphinx in debug mode in my IDE (pycharm) which made the debugging of issues quite painful. If you know a way to achieve that, it could be a nice tip to add in the developer's documentation.

What was the error you were running into? Without pycharm's debugger I would be lost.

@mansenfranzen
Copy link
Owner

I made several small modifications:

  • Added a separate extras in pyproject.toml for erdantic as you've initially implemented it. It makes the user install and the CI setup with erdantic as a dependency both more explicit.
  • Added a separate test running the test suite without installing erdantic since there were no tests without erdantic as a dependency anymore. Since we using tox, the package is build from scratch and installed in clean environment which helps to ensure that build/install process works fine.
  • Updates docs/readme with versions/badges.

@mansenfranzen
Copy link
Owner

Once again, thanks for all the great work you've put into this PR! It was a pleasure working with you :-).

@mansenfranzen mansenfranzen merged commit 55e7aba into mansenfranzen:main Jun 13, 2023
22 of 24 checks passed
Andrew-S-Rosen pushed a commit to Quantum-Accelerators/quacc that referenced this pull request Jul 3, 2023
@dependabot
Bump autodoc-pydantic from 1.8.0 to 1.9.0 (#522)
28de2b7 
Bumps
[autodoc-pydantic](https://github.com/mansenfranzen/autodoc_pydantic)
from 1.8.0 to 1.9.0.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/mansenfranzen/autodoc_pydantic/blob/main/changelog.rst">autodoc-pydantic's
changelog</a>.</em></p>
<blockquote>
<h2>v1.9.0 - 2023-06-08</h2>
<p>This is a feature release adding support for entity relationship
diagrams
while dropping python 3.6. Additionally, pydantic v2 is currently
excluded
until support will be added. Moreover, newest sphinx versions are
added to test matrix.</p>
<p>Feature</p>
<pre><code>
- Introduce ``erdantic-figure`` and ``erdantic-figure-collapsed``
configuration
option for pydantic models to add entity relationship diagrams to
models'
documentation either in collapsed form or as an image included to the
HTML.
`[#148](mansenfranzen/autodoc_pydantic#148)
&lt;https://github.com/mansenfranzen/autodoc_pydantic/pull/148&gt;`__.
<p>Bugfix</p>
<pre><code>
- Run github actions on newest ``ubuntu-22.04``.
- Fix pytest errors with ``sphinx&amp;gt;=6.1`` where the type returned
by
  ``autodoc_typehints_format`` changed.
- Provide upper version boundary for pydantic to exclude v2 which
  is not supported, yet.

Internal
&lt;/code&gt;&lt;/pre&gt;
&lt;ul&gt;
&lt;li&gt;Add &lt;code&gt;to_collapsable&lt;/code&gt; to
&lt;code&gt;directives.templates&lt;/code&gt; that provides a
standardized interface to create a collapsable field.&lt;/li&gt;
&lt;li&gt;Add &lt;code&gt;erdantic&lt;/code&gt; to extras
dependencies.&lt;/li&gt;
&lt;/ul&gt;
&lt;p&gt;Documentation&lt;/p&gt;
&lt;pre&gt;&lt;code&gt;
- Add descriptions for ``erdantic-figure`` and
``erdantic-figure-collapsed``
  options in the configuration section.
- Add an example of ERD in the example section.

Testing
</code></pre>
<ul>
<li>Exclude <code>python 3.6</code> in test matrix.</li>
<li>Include <code>sphinx</code> 6.0, 6.1, 6.2 and 7.0 in test
matrix.</li>
<li>Add tests for <code>erdantic-figure</code> and
<code>erdantic-figure-collapsed</code>.</li>
<li>Fix github actions CI pipeline due to unsupported ubuntu while
upgrading to
newest <code>ubuntu-22.04</code>.</li>
</ul>
<p>Contributors</p>
<pre><code>&amp;lt;/tr&amp;gt;&amp;lt;/table&amp;gt; 
&lt;/code&gt;&lt;/pre&gt;
&lt;/blockquote&gt;
&lt;p&gt;... (truncated)&lt;/p&gt;
&lt;/details&gt;
&lt;details&gt;
&lt;summary&gt;Commits&lt;/summary&gt;

&lt;ul&gt;
&lt;li&gt;&lt;a
href=&quot;mansenfranzen/autodoc_pydantic@10434f76f8abb9faee39eab8f5baf3a7deca558c&quot;&gt;&lt;code&gt;10434f7&lt;/code&gt;&lt;/a&gt;
Fix poetry urllib issue.&lt;/li&gt;
&lt;li&gt;&lt;a
href=&quot;mansenfranzen/autodoc_pydantic@ff0838f4921239e473215b5ad1ee81b6ea4e69e6&quot;&gt;&lt;code&gt;ff0838f&lt;/code&gt;&lt;/a&gt;
Merge pull request &lt;a
href=&quot;https://redirect.github.com/mansenfranzen/autodoc_pydantic/issues/155&quot;&gt;#155&lt;/a&gt;
from mansenfranzen/all-contributors/add-tkaraouzene&lt;/li&gt;
&lt;li&gt;&lt;a
href=&quot;mansenfranzen/autodoc_pydantic@fd83255a8a4808cc4bcc02f825bbbbc28a92c626&quot;&gt;&lt;code&gt;fd83255&lt;/code&gt;&lt;/a&gt;
Add upper pydantic boundary.&lt;/li&gt;
&lt;li&gt;&lt;a
href=&quot;mansenfranzen/autodoc_pydantic@4371c292f4b8f5fe7a735266eb0726f5fb1a5f5c&quot;&gt;&lt;code&gt;4371c29&lt;/code&gt;&lt;/a&gt;
Add contributor.&lt;/li&gt;
&lt;li&gt;&lt;a
href=&quot;mansenfranzen/autodoc_pydantic@97558522dd0e4f3ad603ef29227ed6939a436774&quot;&gt;&lt;code&gt;9755852&lt;/code&gt;&lt;/a&gt;
docs: update .all-contributorsrc [skip ci]&lt;/li&gt;
&lt;li&gt;&lt;a
href=&quot;mansenfranzen/autodoc_pydantic@33629ece29594ec442cb363c9cb885dc36b8809a&quot;&gt;&lt;code&gt;33629ec&lt;/code&gt;&lt;/a&gt;
docs: update README.md [skip ci]&lt;/li&gt;
&lt;li&gt;&lt;a
href=&quot;mansenfranzen/autodoc_pydantic@1605bbcb2b4b458963167c12bb2c92fe4c272789&quot;&gt;&lt;code&gt;1605bbc&lt;/code&gt;&lt;/a&gt;
Merge pull request &lt;a
href=&quot;https://redirect.github.com/mansenfranzen/autodoc_pydantic/issues/154&quot;&gt;#154&lt;/a&gt;
from mansenfranzen/fix_pydantic_v2_incompatibility&lt;/li&gt;
&lt;li&gt;&lt;a
href=&quot;mansenfranzen/autodoc_pydantic@1b61fed303d56f5b82b5bae9d53a1969a3446394&quot;&gt;&lt;code&gt;1b61fed&lt;/code&gt;&lt;/a&gt;
Provide upper version boundary to exclude pydantic v2.&lt;/li&gt;
&lt;li&gt;&lt;a
href=&quot;mansenfranzen/autodoc_pydantic@19ca2ba9d6a76f6338ce0c267761da11f5a63d3c&quot;&gt;&lt;code&gt;19ca2ba&lt;/code&gt;&lt;/a&gt;
Merge pull request &lt;a
href=&quot;https://redirect.github.com/mansenfranzen/autodoc_pydantic/issues/151&quot;&gt;#151&lt;/a&gt;
from mansenfranzen/fix_docs_codecov&lt;/li&gt;
&lt;li&gt;&lt;a
href=&quot;mansenfranzen/autodoc_pydantic@af8a6f249a4784269e7c16951eb005e5ef5c9029&quot;&gt;&lt;code&gt;af8a6f2&lt;/code&gt;&lt;/a&gt;
Test new pytest and coverage versions.&lt;/li&gt;
&lt;li&gt;Additional commits viewable in &lt;a
href=&quot;mansenfranzen/autodoc_pydantic@v1.8.0...v1.9.0&quot;&gt;compare
view&lt;/a&gt;&lt;/li&gt;
&lt;/ul&gt;
&lt;/details&gt;

&lt;br /&gt;
</code></pre>


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=autodoc-pydantic&package-manager=pip&previous-version=1.8.0&new-version=1.9.0)](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>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mansenfranzen mansenfranzen mansenfranzen requested changes

Assignees

@mansenfranzen mansenfranzen

@yves-renier yves-renier

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@yves-renier @mansenfranzen @codecov-commenter