Skip to content

feat(treeplot): lay out the tree without igraph - #7984

Open
kz930 wants to merge 1 commit into
apache:mainfrom
kz930:fix/7969-treeplot-drop-igraph
Open

feat(treeplot): lay out the tree without igraph#7984
kz930 wants to merge 1 commit into
apache:mainfrom
kz930:fix/7969-treeplot-drop-igraph

Conversation

@kz930

@kz930 kz930 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

The Tree Plot operator's generated template no longer imports igraph. It computes its
own top-down layout instead: depth picks the row, a leaf takes the next free column,
and a parent sits centred over its children. Roots are the nodes that never appear as
a child, and anything unreachable from a root is rooted where it is, so a forest, a
cycle, a shared child and a self-loop all place every node exactly once. The walk is
iterative, so a deep tree cannot hit the recursion limit.

igraph was used for four things: Graph.TupleList, vs['name'], layout('rt') and
get_edgelist(). Only the layout did real work, and EdgeSeq was imported but never
used. No new dependency is added.

The layout guard is kept and now reads Tree layout failed. It earns its keep: a cell
like [[1,2], 3] parses to an unhashable node, which now renders the error page
instead of failing the operator.

The tree keeps its shape. Spacing is uniform per leaf rather than Reingold-Tilford's
contour packing, so an unbalanced tree draws slightly wider than before.

Any related issues, documentation, discussions?

Closes #7969. Supersedes #7970, which declared igraph and was closed because GPL v2 is
Category X under the ASF 3rd party license policy.

How was this PR tested?

TreePlotOpDescSpec passes, including a new test that the generated code does not name
igraph. I also dumped the generated template and ran it against a four-level tree with
igraph made unimportable, and it rendered the plot rather than an error page. The root
landed at 2.375 over children at 1.25 and 3.5, their children at 0.5, 2, 3 and 4, and
the deepest pair at 0 and 1. I checked a chain, a forest, a cycle, a diamond, a
self-loop and duplicate edges separately, and a 200k-node chain lays out in 0.29s.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 5)

The generated template imported igraph, which no requirements file declares,
so the operator failed with No module named 'igraph' on any environment built
from the repository. Declaring it is not open to us. igraph is GPL v2, which
is Category X under the ASF 3rd party license policy, and check_binary_deps.py
rejects it.

igraph did four things here: build a graph from the pairs, read the node names
back, run the Reingold-Tilford layout, and return the edge list. Only the
layout does real work, and EdgeSeq was imported but never used. The template
now computes the layout itself. Depth picks the row, a leaf takes the next free
column, and a parent sits centred over its children. Roots are the nodes that
never appear as a child, and anything left over sits in a cycle no root
reaches, so every node is placed exactly once.

The tree keeps its shape. Spacing is uniform per leaf rather than
Reingold-Tilford's contour packing, so an unbalanced tree draws slightly wider
than it did.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @eugenegujing
    You can notify them by mentioning @eugenegujing in a comment.

@codecov-commenter

codecov-commenter commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.14%. Comparing base (7fbf64e) to head (f825865).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #7984   +/-   ##
=========================================
  Coverage     93.14%   93.14%           
+ Complexity     4654     4653    -1     
=========================================
  Files          1174     1174           
  Lines         47580    47580           
  Branches       5316     5316           
=========================================
  Hits          44318    44318           
  Misses         1785     1785           
  Partials       1477     1477           
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø)
agent-service 99.32% <ø> (ø) Carriedforward from 7fbf64e
amber 89.23% <ø> (ø)
computing-unit-managing-service 73.67% <ø> (ø)
config-service 86.73% <ø> (ø)
file-service 85.56% <ø> (ø)
frontend 95.65% <ø> (ø) Carriedforward from 7fbf64e
notebook-migration-service 79.31% <ø> (ø)
pyamber 97.57% <ø> (ø) Carriedforward from 7fbf64e
workflow-compiling-service 77.19% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 0 better · 🔴 5 worse · ⚪ 10 noise (<±5%) · 0 without baseline

Compared against main 7fbf64e benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 413 0.252 22,856/41,434/41,434 us 🔴 +38.7% / 🔴 +204.6%
bs=100 sw=10 sl=64 953 0.581 104,101/134,937/134,937 us ⚪ within ±5% / 🔴 +47.3%
bs=1000 sw=10 sl=64 1,117 0.681 892,749/957,952/957,952 us ⚪ within ±5% / 🔴 +8.7%
Baseline details

Latest main 7fbf64e from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 413 tuples/sec 460 tuples/sec 908.02 tuples/sec -10.2% -54.5%
bs=10 sw=10 sl=64 MB/s 0.252 MB/s 0.281 MB/s 0.554 MB/s -10.3% -54.5%
bs=10 sw=10 sl=64 p50 22,856 us 21,071 us 10,907 us +8.5% +109.6%
bs=10 sw=10 sl=64 p95 41,434 us 29,868 us 13,604 us +38.7% +204.6%
bs=10 sw=10 sl=64 p99 41,434 us 29,868 us 16,697 us +38.7% +148.2%
bs=100 sw=10 sl=64 throughput 953 tuples/sec 963 tuples/sec 1,188 tuples/sec -1.0% -19.8%
bs=100 sw=10 sl=64 MB/s 0.581 MB/s 0.588 MB/s 0.725 MB/s -1.2% -19.8%
bs=100 sw=10 sl=64 p50 104,101 us 102,305 us 85,003 us +1.8% +22.5%
bs=100 sw=10 sl=64 p95 134,937 us 133,478 us 91,599 us +1.1% +47.3%
bs=100 sw=10 sl=64 p99 134,937 us 133,478 us 99,202 us +1.1% +36.0%
bs=1000 sw=10 sl=64 throughput 1,117 tuples/sec 1,121 tuples/sec 1,221 tuples/sec -0.4% -8.5%
bs=1000 sw=10 sl=64 MB/s 0.681 MB/s 0.684 MB/s 0.745 MB/s -0.4% -8.6%
bs=1000 sw=10 sl=64 p50 892,749 us 881,704 us 837,593 us +1.3% +6.6%
bs=1000 sw=10 sl=64 p95 957,952 us 933,809 us 881,015 us +2.6% +8.7%
bs=1000 sw=10 sl=64 p99 957,952 us 933,809 us 915,550 us +2.6% +4.6%
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,484.64,200,128000,413,0.252,22856.32,41434.31,41434.31
1,100,10,64,20,2099.25,2000,1280000,953,0.581,104101.26,134937.39,134937.39
2,1000,10,64,20,17912.45,20000,12800000,1117,0.681,892749.34,957952.13,957952.13

@kz930

kz930 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

/request-review @aglinxinyuan @carloea2

kz930 added a commit to Nicoleee1108/texera_workflow_to_py that referenced this pull request Aug 26, 2026
The generated template imported igraph, which no requirements file declares,
so the operator failed with No module named 'igraph' on any environment built
from the repository. igraph is GPL v2, Category X under the ASF 3rd party
license policy, so declaring it is not open to us.

Both generators now compute the layout themselves. Depth picks the row, a leaf
takes the next free column, and a parent sits centred over its children. Roots
are the nodes that never appear as a child, and anything left over sits in a
cycle no root reaches, so every node is placed exactly once.

The template half is the same change as apache#7984. The standalone
half has no upstream counterpart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The Tree Plot operator can never run: its igraph dependency is Category X

2 participants