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

Fix Digraph Graphics #224

Merged
merged 2 commits into from
Dec 31, 2022
Merged

Fix Digraph Graphics #224

merged 2 commits into from
Dec 31, 2022

Conversation

mepland
Copy link
Collaborator

@mepland mepland commented Dec 30, 2022 •
edited
Loading

  • Changed additional hard coded Helvetica to fontname in digraph.
  • Added fontname="{fontname}" for digraph edges, now all fonts should be the same.
  • Added leading spaces to all_llabel, all_rlabel and root_llabel, root_rlabel.
  • Noted that the "  " in edge_label is not doing anything for pdf views, see attached.

dtreeviz_pred_path
dtreeviz_pred_path.pdf

@mepland
Changed additional hard coded Helvetica to fontname in digraph.
9e884af 
Added leading spaces to all_llabel, all_rlabel and root_llabel, root_rlabel.
Noted that the " &parrt#160;" in edge_label is not doing anything for pdf views.
Added fontname="{fontname}" for digraph edges, now all fonts should be the same.
@mepland
Standardize leading white space for edge labels
7b14d25
@mepland
Copy link
Collaborator Author

mepland commented Dec 30, 2022

Hmm actually looking at it again, the "Prediction 0" text is in the correct spot on the png I save, but not the pdf, so I think that it is fine. I just changed to two leading spaces to match the other labels. The   character doesn't appear to be doing anything in my testing.

Ideally we would just move the edge label positions in the graph itself, but this doesn't appear possible for graphviz:

  • Ref 1
  • Ref 2
  • Ref 3
  • Ref 4 - Note labeldistance only works for headlabel and taillabel, not the midpoint label we use.

@mepland mepland mentioned this pull request Dec 30, 2022
Closed
@parrt parrt added the enhancement New feature or request label Dec 30, 2022
@parrt parrt added this to the 2.1 milestone Dec 30, 2022
@parrt
Copy link
Owner

parrt commented Dec 30, 2022

Ideally we would just move the edge label positions in the graph itself, but this doesn't appear possible for graphviz:

yeah, I had lots of trouble with positioning...

@parrt parrt mentioned this pull request Dec 30, 2022
Closed
@mepland mepland mentioned this pull request Dec 31, 2022
Closed
@parrt parrt merged commit a79aa94 into parrt:dev Dec 31, 2022
@parrt
Copy link
Owner

parrt commented Dec 31, 2022

just ran notebooks. looks fine. thanks!

@mepland mepland deleted the fix_graphics branch January 14, 2023 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@parrt parrt parrt left review comments

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
2.1
Development

Successfully merging this pull request may close these issues.
2 participants
@mepland @parrt