Warn on unused visualization kwargs that only apply to FancyArrowPatch edges #6098
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rossbar
added
type: Enhancements
Visualization
jarrodmillman
approved these changes
Oct 20, 2022
Mjh9122
pushed a commit
to Mjh9122/networkx
that referenced
this pull request
Oct 22, 2022
…h edges (networkx#6098) * Add test for UserWarning with ignored vizopts. * Implement UserWarning for bad vizopts combos. * Fix instance in test suite caught by new warnings. * Update test to check warnings are *not* raised when using FAPs.
MridulS
pushed a commit
to MridulS/networkx
that referenced
this pull request
Feb 4, 2023
…h edges (networkx#6098) * Add test for UserWarning with ignored vizopts. * Implement UserWarning for bad vizopts combos. * Fix instance in test suite caught by new warnings. * Update test to check warnings are *not* raised when using FAPs.
cvanelteren
pushed a commit
to cvanelteren/networkx
that referenced
this pull request
Apr 22, 2024
…h edges (networkx#6098) * Add test for UserWarning with ignored vizopts. * Implement UserWarning for bad vizopts combos. * Fix instance in test suite caught by new warnings. * Update test to check warnings are *not* raised when using FAPs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #5857 and #5694
draw_networkx_edgeshas a few keyword arguments that only apply when FancyArrowPatch (FAP) objects are used to represent the edges (as opposed to a LineCollection (LC)). The tricky part is that the logic for determining which object is used is not immediately clear to the user. By default, LCs are used when the input graphGis undirected, and FAPs are used whenGis directed. The reason for this is that LC are much faster to draw than FAPs, so using LCs where possible greatly enhances the ability to visualize graphs with many edges. The default behavior can be overridden with thearrowskwarg:arrows=Truewill forcedraw_networkx_edgesto use FAPs even ifGis undirected. This may make the visualization much slower, especially if there are a lot of edges, but the tradeoff is that the user can then take advantage of FAP-specific features, like the ability to draw curved edges.This PR introduces UserWarnings whenever a user calls
draw_networkx_edgeswith a FAP-specific kwarg when LC is being used to draw the edges. The current behavior is to silently ignore the FAP-specific kwarg which leads to confusion - see e.g. #5857 and #5694. The warning message specifies how to fix the situation, either by usingarrows=Trueto force FAPs (which may be slow) or by deleting the unused kwarg option if it is not needed.The hope is that the warning is useful without being noisy - note that adding it in caught one instance in the NX test suite where kwargs were silently ignored. The other concern is that the warnings are not raised when they shouldn't be - I double checked the output of the gallery examples to ensure that there were no new, unexpected warnings from this change.