Skip to content

Tighten flow charts more - #137

Merged
joezuntz merged 4 commits into
masterfrom
more-flowchart-tightening
Oct 6, 2026
Merged

joezuntz merged 4 commits into
masterfrom
more-flowchart-tightening

Conversation

@joezuntz

@joezuntz joezuntz commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

This combines input nodes that can all be put together in a plot.

Also apparently the networkx method nodes_iter has been removed in more recent versions and been replaced with just nodes, which now returns a view. This shouldn't matter for older versions where nodes() returns a list, since we are dealing with small graphs here so there will be no memory issues.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The new test passes a NetworkX graph to a PyGraphviz-specific helper, causing an AttributeError before its assertions.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Makes pipeline flow charts more compact by combining eligible input nodes.

Changes:

  • Groups input nodes with no predecessors and the same sole successor.
  • Integrates grouping into flow-chart generation and replaces nodes_iter() with nodes().
  • Adds a test for input-node merging.
File Description
tests/​test_graph.py Adds an input-node merging test.
ceci/​pipeline/​pipeline.py Applies input grouping and updates node iteration.
ceci/​pipeline/​graph.py Implements input grouping and updates output-node iteration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/test_graph.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The focused chart changes use supported APIs, include regression coverage, and have no unresolved blocking findings.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@joezuntz
joezuntz merged commit ab3968f into master Oct 6, 2026
9 checks passed
@joezuntz
joezuntz deleted the more-flowchart-tightening branch October 6, 2026 10:23
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.

2 participants