-
Notifications
You must be signed in to change notification settings - Fork 159
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
mapping is not possible when a node has no neighbors #995
Conversation
Pull Request Test Coverage Report for Build 6431767936
💛 - Coveralls |
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.
This LGTM, just one inline comment that I think will be more efficient.
rustworkx-core/src/token_swapper.rs
Outdated
.neighbors(id_node) | ||
.collect::<Vec<G::NodeId>>() | ||
.is_empty() |
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.
.neighbors(id_node) | |
.collect::<Vec<G::NodeId>>() | |
.is_empty() | |
.neighbors(id_node) | |
.next() | |
.is_some() |
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.
Oh, this is how one can directly check whether an iterator is empty or non-empty, I was wondering about that. Fixed in d682b9b.
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.
LGTM, thanks for the quick update and also fixing the mistake in my suggestion :D
Fixes #994.
I believe that the function
add_token_edges(node)
should returnErr(MapNotPossible {})
when no mapping exists. This was already handled in the case thatnode
had at least one neighbor. Now I am also returning the error in the case a node has no neighbors.