Skip to content

Change graphviz font family to sans#7931

Merged
martindurant merged 5 commits intodask:mainfrom
freyam:graphviz-fontface
Jul 28, 2021
Merged

Change graphviz font family to sans#7931
martindurant merged 5 commits intodask:mainfrom
freyam:graphviz-fontface

Conversation

@freyam
Copy link
Copy Markdown
Contributor

@freyam freyam commented Jul 24, 2021

  • Tests passed
  • Passes black dask / flake8 dask / isort dask

I was working on #7869 and realized that the Graphviz output would look even cleaner if we use a sans-serif font instead of the current serif font.

In this PR, I have changed the font of the Graphviz from Times-Roman to Roboto Helvetica. I have also cleaned the code in the method to_graphviz() to be more consistent in dot.py (LOW LEVEL) and highlevelgraph.py (HIGH LEVEL). Now, they look similar and it might be easier to work with this code in the future.

It follows the https://marketing.dask.org/en/latest/colors.html
dask
(Font mentioned in image: Roboto)

Even the HTML Representation uses a Sans font, so it makes sense to have the Graph representations also follow suit.

Demo

import dask.array as da

x = da.ones((10, 10), chunks=(5, 5))
x = x + 100
x = x * 100

x.visualize()

Before

b1

After

a1

import dask.array as da

XL = da.ones((67552, 365941), chunks=(652, 5792))
XL2 = XL.T.astype("f4")
XC = da.ones((365941, 26), chunks=(365941, 26))
LS = da.linalg.lstsq(XC, XL2)[0]
XC_LS = XC @ LS
XLP = XL2 - XC_LS

XLP.dask.visualize(\)

Before

b2

After

a2

/cc @GenevieveBuckley @martindurant

@GenevieveBuckley
Copy link
Copy Markdown
Contributor

My personal opinion here is the same as described in #7869 (comment)

I'm mildly supportive of the suggestion to use a sans serif font for consistency with other representations of the high level graph, but I wouldn't die on this hill if other people dislike it. Now that the font size is often a lot bigger, it certainly seems more like a bunch of headings (where sans serif is typically expected) rather than paragraph-like text.

@martindurant
Copy link
Copy Markdown
Member

I am also +1 here, so lets merge in a day if there is no dissent.

Copy link
Copy Markdown
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @freyam!

@freyam
Copy link
Copy Markdown
Contributor Author

freyam commented Jul 28, 2021

This is ready to merge, if no one has any issues.

/cc @martindurant @jrbourbeau

@martindurant martindurant merged commit 5fbbf90 into dask:main Jul 28, 2021
@freyam freyam deleted the graphviz-fontface branch July 28, 2021 13:11
@freyam
Copy link
Copy Markdown
Contributor Author

freyam commented Jul 28, 2021

💛

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants