Skip to content

Conversation

@networmix
Copy link
Owner

Summary

  • allow selecting node groups by attribute value using attr: directive
  • document attribute directive in network view selection
  • test attribute-based grouping for networks and views

Testing

  • make format (fails: pre-commit: No such file or directory)
  • ruff format ngraph/network.py ngraph/network_view.py tests/test_network_selection.py tests/test_network_view.py
  • ruff check ngraph/network.py ngraph/network_view.py tests/test_network_selection.py tests/test_network_view.py --fix
  • make docs
  • make check (fails: pre-commit is not installed)
  • pytest (fails: No module named 'yaml')

https://chatgpt.com/codex/tasks/task_e_6894b36acdf0832186d9aed47ad47219

Copilot AI review requested due to automatic review settings August 7, 2025 16:21
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 adds support for selecting and grouping nodes by attribute values using an attr: directive prefix in the select_node_groups_by_path method. This extends the existing regex-based node selection to allow grouping by arbitrary node attributes.

Key changes:

  • Added attribute-based node grouping with attr:<name> syntax
  • Updated docstrings to document the new directive
  • Added comprehensive test coverage for both Network and NetworkView classes

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
ngraph/network.py Added attribute-based grouping logic and updated docstring
ngraph/network_view.py Updated docstring to document attribute directive
tests/test_network_selection.py Added test for attribute-based grouping on Network
tests/test_network_view.py Added test for attribute-based grouping on NetworkView

Comment on lines +246 to +247
value = node.attrs.get(attr_name)
if value is not None:
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

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

The condition value is not None excludes nodes with explicit None values, which may be valid attribute values. Consider using attr_name in node.attrs to check for attribute existence instead, or document this behavior clearly.

Suggested change
value = node.attrs.get(attr_name)
if value is not None:
if attr_name in node.attrs:
value = node.attrs[attr_name]

Copilot uses AI. Check for mistakes.
@networmix networmix closed this Aug 8, 2025
@networmix networmix deleted the investigate-groupby-implementation-for-netgraph branch August 8, 2025 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants