Skip to content

Conversation

@sbrugman
Copy link
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings November 25, 2025 13:28
@sbrugman sbrugman added the fix Something isn't working label Nov 25, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the codebase to use Fully Qualified Names (FQNs) more consistently, particularly in the visualization package. The changes improve code organization by consolidating node and IO processing logic and standardizing how FQNs are handled across the codebase.

Key changes:

  • Consolidated node and IO processing into a new process_nodes_and_ios function in _process_nodes_and_ios.py
  • Updated visualization code to use the FQN class instead of the fqn_to_object_ref function for better type safety and clarity
  • Removed import aliases in test examples to ensure proper FQN resolution (e.g., b as Bb)

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/ordeq/src/ordeq/_runner.py Refactored to use new process_nodes_and_ios function, removing inline validation and processing logic
packages/ordeq/src/ordeq/_resolve.py Added _validate_runnables function (moved from _runner.py) for better code organization
packages/ordeq/src/ordeq/_process_nodes_and_ios.py Added new process_nodes_and_ios function that consolidates runnable validation, resolution, and processing
packages/ordeq-viz/src/ordeq_viz/graph.py Updated to use FQN class and properly handle FQNs from nodes, with type annotation added to _add_io_data parameter
packages/ordeq-viz/src/ordeq_viz/api.py Updated to use process_nodes_and_ios function instead of separate processing steps
packages/ordeq-test-examples/src/example_project/nodes_import_alias.py Removed import alias (b as Bb) to ensure correct FQN resolution
packages/ordeq/tests/snapshots/runner/*.snapshot.md Updated stack traces to reflect new function call structure
packages/ordeq-viz/tests/snapshots/**/*.snapshot.md Updated visualizations to show correct FQNs (e.g., b instead of B, example_3.nodes:f1 instead of example_3.func_defs:hello)

@sbrugman sbrugman enabled auto-merge (squash) November 25, 2025 13:54
@sbrugman sbrugman changed the title ordeq-viz: use FQNs ordeq-viz: use FQNs for nodes Nov 25, 2025
@sbrugman sbrugman disabled auto-merge November 25, 2025 13:55
@sbrugman sbrugman enabled auto-merge (squash) November 25, 2025 13:55
@sbrugman sbrugman changed the title ordeq-viz: use FQNs for nodes ordeq-viz: use FQNs for nodes and IOs Nov 25, 2025
@sbrugman sbrugman disabled auto-merge November 25, 2025 15:27
@sbrugman sbrugman enabled auto-merge (squash) November 25, 2025 15:27
@sbrugman sbrugman merged commit f3d49a9 into main Nov 25, 2025
40 checks passed
@sbrugman sbrugman deleted the ordeq-viz-fqns branch November 25, 2025 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants