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

Feature: Customizable graph label -> remove erdantic watermark #78

Closed
mansenfranzen opened this issue May 2, 2023 · 4 comments · Fixed by #106
Closed

Feature: Customizable graph label -> remove erdantic watermark #78

mansenfranzen opened this issue May 2, 2023 · 4 comments · Fixed by #106
Labels
enhancement New feature or request
Milestone

Comments

@mansenfranzen
Copy link

First of all, thanks for creating erdantic in the first place! I really like how it's possible to generate ERDs from pydantic models and ouput them via SVG.

I just really wish it would be possible to provide custom labels for the resulting graphs. Or at least to provide a possibility to remove the erdantic watermark Created by erdantic v0.5.0 <https://github.com/drivendataorg/erdantic>.

From my personal view, this is a drawback as it distracts the viewer from the main content. Moreover, it feels like the resulting diagram was generated with some shareware or free license of a proprietary software which requires self credits. Actual, the total opposite is the case because you've chosen MIT license (thanks for that!). I strongly believe that your package does not require the watermark and would enjoy even wider usage without it.

Currently, I hesitate to include it into autodoc_pydantic without any customization. @yves-renier has created a great PR to leverage erdantic improving the auto-documentation of pydantic models.

I would be willing to provide a PR upstream adding this feature if you agree.

@jayqi
Copy link
Member

jayqi commented May 4, 2023

Hi @mansenfranzen. Thanks for the feedback. As an immediate workaround without any changes to erdantic, you could generate the pygraphviz object and then modify its attributes to remove the label, or generate the graphviz DOT language specification of your graph, and modify the DOT to remove the label.

I'm open to making this customizable and would like to think a little more about the API design.

@mansenfranzen
Copy link
Author

Hi @jayqi,

thanks for taking your time looking at this issue!

Regarding an immediate workaround, I'm feeling a bit reluctant to put any pygraphviz logic into autodoc_pydantic because it does not belong there.

I would be more than happy to encapsulate everything required within erdantic. If possible, I'm happy to assist you but I know API design considerations take time ;-).

From my point of view, the draw and to_dot functions/methods should both support specifying the label. The EntityRelationshipDiagram.graph method generates the pygraphviz object and will require access to the label information.

Actually while thinking about EntityRelationshipDiagram.graph, nothing can be configured for the pygraphviz object at the moment. I think it would be a great feature to pass an optional configuration dictionary to the EntityRelationshipDiagram.graph method to overwrite the default values passed to the pgv.AGraph object instantiation. This would not only allow to overwrite the label but also all other attributes (such as font size, color, etc...). This would provide great flexibility to your users. What do you think?

Coming back to the API consideration, the draw and to_dot functions/methods could support a new parameter named graph_config which communicates a clear purpose and is unambiguous in my opinion. Moreover, this approach will prevent bloating up the signatures of the draw and to_dot functions/methods for any future feature requests regarding the customizability of the pgv.AGraph object.

@jayqi
Copy link
Member

jayqi commented May 10, 2023

Hi @mansenfranzen,

Thanks for following up and for taking the time to look at the code and think about the design.

I think your suggestion for supporting keyword arguments or a dictionary that is passed through to the AGraph instantiation makes sense. (My preference would be to name this something like graph_kwargs, which feels like it signposts what it is a little more explicitly.)

If you are willing to contribute a PR, that would be welcome.


One design choice is whether graph() should directly pass through keyword arguments (with **kwargs) or whether it should be a dictionary, like you suggested. (We're locked in already with draw() since that already does keyword argument passthrough to AGraph.draw.)

#74 may be relevant here—that had mentioned passing through node and edge configuration.

@jayqi jayqi added the enhancement New feature or request label Feb 26, 2024
@jayqi jayqi added this to the v1.0 rewrite milestone Feb 26, 2024
@jayqi
Copy link
Member

jayqi commented Mar 31, 2024

Hi @mansenfranzen,

Customizing the graph label is available in the new v1.0.0. This is currently a pre-release version (v1.0.0rc1), installable with pip install erdantic --pre. Your testing and feedback would be much appreciated.

There are new graph_attr, node_attr, and edge_attr parameters on the draw and to_graphviz methods/functions to pass in these Graphviz attributes. They get merged into the respective mapping on the created Graphviz object. (to_graphviz is the replacement for the old graph property`.)

You can see documentation regarding this functionality here: https://erdantic.drivendata.org/v1.0/customizing/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants