-
-
Notifications
You must be signed in to change notification settings - Fork 206
feat: error when adding named vertices to non-empty unnamed graphs #2452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: krlmlr <[email protected]>
Co-authored-by: krlmlr <[email protected]>
krlmlr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot: Abort instead of warning. Adding to an empty graph should work. Remember to add a snapshot test for the error message.
- Changed from cli_warn to cli_abort for non-empty unnamed graphs - Allow adding named vertices to graphs with no edges (empty graphs) - Added snapshot tests for error messages - Removed suppressWarnings from tests that should now work Co-authored-by: krlmlr <[email protected]>
Changed to abort instead of warn in commit 8723eee. The implementation now:
The key insight is checking |
krlmlr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot: Empty graphs have zero vertices, not edges. Review the error message.
- Changed check from ecount(e1) > 0 to vcount(e1) > 0 - Empty graphs are now correctly defined as having 0 vertices, not 0 edges - Updated tests to use make_empty_graph() instead of make_empty_graph(1) - Added tests for make_empty_graph(1) to verify it now errors correctly Co-authored-by: krlmlr <[email protected]>
Corrected in commit a4aa43d. Changed the check from |
Adding named vertices to unnamed graphs creates confusing output where existing vertices have
NAnames and edges display asNA--NA.Changes
+.igraphwhen:vertex()/vertices()with unnamed arguments (interpreted as names) on non-empty unnamed graphs (graphs with vertices)g + c("a", "b")) on non-empty unnamed graphsmake_empty_graph()) is allowed, enabling graph construction from scratchtests/testthat/_snaps/operators.mdThe implementation checks
vcount(e1) > 0to determine if a graph is non-empty. Empty graphs are defined as having zero vertices, consistent with standard graph theory terminology.Example
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.