Make font size dynamic

[e1ha]

Describe your changes

make font size dynamic based on width of the the tasks/circles in the dag plot

Issue ticket number and link

Closes #903

Checklist before requesting a review

• I have performed a self-review of my code
• I have added thorough tests (when necessary).
• I have added the right documentation (when needed). Product update? If yes, write one line about this update.

[edublancas]

thanks, this works! I realized the font gets very small when the name gets very long (which is expected, based on the description on the issue). I didn't consider this initially.

How difficult it is to increase the width of the circle dynamically (turn it into an oval) so the font doesn't get too small.

similar to this:

[e1ha]

Increasing the width of the circle dynamically is easy but it gets messy when the ovals overlap. I think I would need to recalculate the positions of the arrows and the circles since as it is now, those positions seem to be based on the current radius size of the circle.

[edublancas]

Understood. I think we have two options:

1. keep the current solution
2. instead of changing the font size, we can add some background color so even if the label overflows, the label is readable

thoughts @idomic ?

[idomic]

For 2. I don't think it'll be necessary, black font on white background is fine.

I think let's try it dynamically, if the text overflows, enlarge the width, if overlapping add a new line and add fixed size to the height. I think that should fix the issues.

Also, if we had a sample version to test it'd make this discussion way easier.

[e1ha]

I sent you a sample version with dynamic width on slack.

[idomic]

Connected on slack, waiting for a sample PR.

[edublancas]

For 2. I don't think it'll be necessary, black font on white background is fine.

the problem is that there are two backgrounds. if the black text overflows, part of the background will be white but the other will be red/green

[idomic]

@yafimvo thoughts on making this dynamic?
If no ideas, we can take the approach of having the text grey background color instead, so at least it's readable.

[yafimvo]

@yafimvo thoughts on making this dynamic? If no ideas, we can take the approach of having the text grey background color instead, so at least it's readable.

1. Maybe we can wrap the long text so it will fit to the circle
   Found these
   https://bl.ocks.org/mbostock/7555321
   https://observablehq.com/@mbostock/fit-text-to-circle

2. Another possible solution (less esthetic though) is to highlight the text so that even if it will overflow it will keep color contrast. We can do that by adding rectangle before appending the text.

[e1ha]

For this commit, I didn't end up changing the circle's shape since I thought that fitting the task name in the circle should be enough space for most cases.

[idomic]

@e1ha is this the latest code? and is this ready for review?

[e1ha]

yes it is

[e1ha]

From what I see in the CI/CD pipeline the issue is AttributeError: module 'mistune' has no attribute 'Renderer'. I don't believe that's caused by my commit.

[idomic]

Try rebasing? We updated mistune a week ago maybe it's related..

[idomic]

I see a flake8 error on the code that doesn't exist.
Let's open a new PR and cherry-pick your changes, that should fix that out.

[edublancas]

From what I see in the CI/CD pipeline the issue is AttributeError: module 'mistune' has no attribute 'Renderer'. I don't believe that's caused by my commit.

yeah i fixed this recently

I see a flake8 error on the code that doesn't exist.

upgrade your local flake8. i saw this a few weeks ago

[e1ha]

It looks like one of the tests failed in the CI/CD pipeline: FAILED tests/test_micro.py::test_capture[None-True] - Failed: Timeout >120.0s . What should I do?

[edublancas]

yeah, I saw them as well. I still don't know why they're failing. in the master branch, we're skipping them so they don't cause trouble, if you rebased and still see them you can ignore them

[grnnja]

Just curious is there a reason why we are rolling our own dag plotter with d3 instead of using something off the shelf?

[idomic]

something off the shelf like what?

[grnnja]

something off the shelf like what?

I couldn't really find anything better. It just seems weird that we would have to handle font sizing ourselves since I'd imagine it's a problem other people have run into.

[edublancas]

@grnnja we're using a library: https://erikbrinkman.github.io/d3-dag/

maybe we missed something obvious and forgot to check if that library supports adding text to the nodes (and takes care of wrapping text)? 😅

[idomic]

@e1ha I think it's a good point, can you do a short research on what built-in functionality we can leverage here? (instead of reinventing the wheel)

[e1ha]

It's done now.

[idomic]

@e1ha I just tested it.
I think overall it's good, 3 comments on the photo here.

1. We should center (height/width) the text to the circle.
2. There's a bit of a gap in the top of the bottom circle (with tons of text), so in that case, there's underutilized space on top of it and the text is overflowing on the bottom of it.
3. I saw a commented block above your code, is that the previous code? Let's remove this commented block.

Please fix those and let's merge

[e1ha]

I saw a commented block above your code, is that the previous code? Let's remove this commented block

The commented block is not my code. Should I still remove it?

There's a bit of a gap in the top of the bottom circle (with tons of text), so in that case, there's underutilized space on top of it and the text is overflowing on the bottom of it.

Would you mind if the text sticks out of the circle at the top of the circle? Similar to what happens to that last circle.

[yafimvo]

@edublancas Fixed long text issue and the text position within a node.

[edublancas]

looks like the full name isn't visible:

HTML attached:

pipeline.html.zip

this is the pipeline.yaml:

tasks:
    # tasks.get, features and join are python functions
  - source: tasks.get
    name: this-is-a-really-long-name-get
    product: output/get.parquet

  - source: tasks.features
    name: this-is-a-really-long-name-features
    product: output/features.parquet

  - source: tasks.join
    name: this-is-a-really-long-name-join
    product: output/join.parquet

    # fit.py is a script (that you can open as notebook in Jupyter)
  - source: fit.py
    name: this-is-a-really-long-name-fitfitfitfitfitfit
    product:
        # this output notebook is the model's evaluation report
        nb: output/nb.html
        model: output/model.pickle

    # optional config to hide the code in the output/nb.html report
    nbconvert_export_kwargs:
      exclude_input: True

[yafimvo]

I added a wrapper that should trim long text that doesn't fit into the circle's height and width without affecting the readability.