Skip to content

Issue #24 - Use ELK layout to position SVG tasks + links - #30

Open
LudoBroche wants to merge 9 commits into
mainfrom
24-feature-use-elk-layout-to-position-svg-tasks
Open

Issue #24 - Use ELK layout to position SVG tasks + links#30
LudoBroche wants to merge 9 commits into
mainfrom
24-feature-use-elk-layout-to-position-svg-tasks

Conversation

@LudoBroche

@LudoBroche LudoBroche commented Aug 28, 2026

Copy link
Copy Markdown
Member

PR summary

From the graph/tasks geometry ELK representation, we now used ELK to compute the tasks' optimal positions and link routing. The computed ELK layout is used to position the tasks and draw the links.

AI Disclosure

  • Claude used to pre-review the PR
  • Claude used to give advice on the pytest coverage plan.
  • Claude used to render the svg collage of all the workflow svgs in the PR comment

@LudoBroche LudoBroche linked an issue Aug 28, 2026 that may be closed by this pull request
@LudoBroche LudoBroche changed the title Issue #24 - First implementation - tasks positioning + links display Issue #24 - Use ELK layout to position SVG tasks + links Aug 28, 2026
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.91667% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/ewoksdraw/svg/svg_task_group.py 83.33% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@LudoBroche
LudoBroche force-pushed the 24-feature-use-elk-layout-to-position-svg-tasks branch from 036190d to dd4db3c Compare August 28, 2026 15:44
Comment thread src/ewoksdraw/layout/elk_converter.py
Comment thread src/ewoksdraw/layout/elk_link_group_builder.py
Comment thread src/ewoksdraw/layout/elk_link_group_builder.py
Comment thread src/ewoksdraw/tests/test_graph_to_svg.py
Comment thread src/ewoksdraw/__init__.py Outdated
Comment thread src/ewoksdraw/__init__.py Outdated
@LudoBroche

Copy link
Copy Markdown
Member Author

Tested with ewoks examples:

for graph in acyclic1 acyclic2 acyclic3 cyclic1 demo empty self_trigger triangle1
do
    pixi run graph-to-svg --test "$graph"
done

Seems to be working nicely.

all_workflows

@LudoBroche LudoBroche added this to the ewoksdraw 1.0 release milestone Aug 28, 2026
@LudoBroche
LudoBroche requested a review from loichuder August 28, 2026 17:01
Comment thread src/ewoksdraw/layout/elk_converter.py Outdated
Comment thread src/ewoksdraw/layout/elk_converter.py
Comment thread src/ewoksdraw/layout/elk_converter.py
Comment thread src/ewoksdraw/layout/elk_link_group_builder.py
Comment thread src/ewoksdraw/layout/elk_link_group_builder.py Outdated
Comment thread src/ewoksdraw/layout/elk_link_group_builder.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure this is the best place nor the best name for this module.

Should it not be in svg ? Or even put the function in svg_task_group?

@LudoBroche LudoBroche Sep 4, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, it's a converter from: ewoks graph -> SVG
In the same module we have: elk_link_group_builder.py elk graph -> SVG and elk_converter.py SVG -> elk graph.

I don't think it should be svg_task_group, since it handles Ewoks graphs.
I can create a new module for it. What do you think?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ewoksdraw/ewoks_to_svg ?

Comment thread src/ewoksdraw/svg/svg_task_group.py Outdated
Comment thread src/ewoksdraw/tests/test_graph_to_svg.py Outdated
Comment thread src/ewoksdraw/tests/test_graph_to_svg.py Outdated
@LudoBroche
LudoBroche requested a review from loichuder September 4, 2026 15:04
@LudoBroche

LudoBroche commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

@loichuder I apply your suggestions.
Two points are still pending:

  1. ewoks_task_group_builder.py
  2. The new API location. I tried to follow the Ewoks' project style by creating a binding.py. Let me know what you think.

@loichuder

Copy link
Copy Markdown
Member

@loichuder I apply your suggestions. Two points are still pending:

1. [ewoks_task_group_builder.py](https://github.com/ewoks-kit/ewoksdraw/pull/30#discussion_r3903867146)

2. The new API location. I tried to follow the Ewoks' project style by creating a `binding.py.` Let me know what you think.

Above I suggested ewoks_to_svg as the new home for ewoks_task_group_builder. It can also be the home of graph_to_svg then?

@loichuder loichuder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!


class ElkChild(TypedDict):
class ElkChildBeforeLayout(TypedDict):
"""An ELK child before layout."""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"""An ELK child before layout."""

🙃

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.

[Feature]: Use ELK layout to position SVG tasks

2 participants