Skip to content

Commit

Permalink
Fixed clippy warnings (#627)
Browse files Browse the repository at this point in the history
I ran cargo clippy and fixes most but not all of the warnings.

I manualy reviewed all the changes ... they all look like  improvements.

Nothing earth shaking lots of tiny blemishes removed.

By was of testing, I can asset that 
```
cargo test feature --all
```

passes.

When I say most but not all clippy warning are cleared. A warning of
this form remains

```warning: docs for unsafe trait missing `# Safety` section```

but I think documentation issues are best dealt with in a separate issue 

Highlights :-

1) .any() is prefered over .find(), as it will break a "loop" on the first occurance.
```
- if let Some(_) = w_out_edges.find(|e| e.target() == nodeix(v)) {
+ if w_out_edges.any(|e| e.target() == nodeix(v)) {
```

2) Was manually implementing a call to .find()

```
-        while let Some(edge) = self.edges.next() {
-            if edge.node[1] == self.target_node {
-                return Some(edge);
-            }
-        }
-
-        None
+      let target_node = self.target_node;
+      self.edges.by_ref().find(|&edge| edge.node[1] == target_node)
```

3) Made find_join() make more flexible by accepting slices rather
   than a strict vec.

```
-    label: &mut Vec<Label<G>>,
-    first_inner: &mut Vec<usize>,
+    label: &mut [Label<G>],
+    first_inner: &mut [usize],
```

4) Now using flap_map where possible
```
-        .map(|(i, &node)| {
+        .flat_map(|(i, &node)| {
```

5) using .by_ref() to repalce a while loop with something
   more idomatic.

```
-        while let Some(next) = self.iter.next() {
+        for next in self.iter.by_ref() {
```

6) removed the use of a deprecated function.

```
-        let weight = self.edges.remove(&Self::edge_key(a, b));
+        let weight = self.edges.swap_remove(&Self::edge_key(a, b));
```
  • Loading branch information
martinfrances107 committed Apr 11, 2024
1 parent 3361e04 commit 5cabfe3
Show file tree
Hide file tree
Showing 17 changed files with 32 additions and 39 deletions.
6 changes: 3 additions & 3 deletions benches/bellman_ford.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use petgraph::algo::{bellman_ford, find_negative_cycle};
fn bellman_ford_bench(bench: &mut Bencher) {
static NODE_COUNT: usize = 100;
let mut g = Graph::new();
let nodes: Vec<NodeIndex<_>> = (0..NODE_COUNT).into_iter().map(|i| g.add_node(i)).collect();
let nodes: Vec<NodeIndex<_>> = (0..NODE_COUNT).map(|i| g.add_node(i)).collect();
for i in 0..NODE_COUNT {
let n1 = nodes[i];
let neighbour_count = i % 8 + 3;
Expand All @@ -25,7 +25,7 @@ fn bellman_ford_bench(bench: &mut Bencher) {
if n1 != n2 {
distance -= 1.0
}
g.add_edge(n1, n2, distance as f64);
g.add_edge(n1, n2, distance);
}
}

Expand All @@ -38,7 +38,7 @@ fn bellman_ford_bench(bench: &mut Bencher) {
fn find_negative_cycle_bench(bench: &mut Bencher) {
static NODE_COUNT: usize = 100;
let mut g = Graph::new();
let nodes: Vec<NodeIndex<_>> = (0..NODE_COUNT).into_iter().map(|i| g.add_node(i)).collect();
let nodes: Vec<NodeIndex<_>> = (0..NODE_COUNT).map(|i| g.add_node(i)).collect();
for i in 0..NODE_COUNT {
let n1 = nodes[i];
let neighbour_count = i % 8 + 3;
Expand Down
2 changes: 1 addition & 1 deletion benches/dijkstra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use petgraph::algo::dijkstra;
fn dijkstra_bench(bench: &mut Bencher) {
static NODE_COUNT: usize = 10_000;
let mut g = Graph::new_undirected();
let nodes: Vec<NodeIndex<_>> = (0..NODE_COUNT).into_iter().map(|i| g.add_node(i)).collect();
let nodes: Vec<NodeIndex<_>> = (0..NODE_COUNT).map(|i| g.add_node(i)).collect();
for i in 0..NODE_COUNT {
let n1 = nodes[i];
let neighbour_count = i % 8 + 3;
Expand Down
2 changes: 1 addition & 1 deletion benches/floyd_warshall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use petgraph::algo::floyd_warshall;
fn floyd_warshall_bench(bench: &mut Bencher) {
static NODE_COUNT: usize = 100;
let mut g = Graph::new_undirected();
let nodes: Vec<NodeIndex<_>> = (0..NODE_COUNT).into_iter().map(|i| g.add_node(i)).collect();
let nodes: Vec<NodeIndex<_>> = (0..NODE_COUNT).map(|i| g.add_node(i)).collect();
for i in 0..NODE_COUNT {
let n1 = nodes[i];
let neighbour_count = i % 8 + 3;
Expand Down
2 changes: 1 addition & 1 deletion benches/k_shortest_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use petgraph::algo::k_shortest_path;
fn k_shortest_path_bench(bench: &mut Bencher) {
static NODE_COUNT: usize = 10_000;
let mut g = Graph::new_undirected();
let nodes: Vec<NodeIndex<_>> = (0..NODE_COUNT).into_iter().map(|i| g.add_node(i)).collect();
let nodes: Vec<NodeIndex<_>> = (0..NODE_COUNT).map(|i| g.add_node(i)).collect();
for i in 0..NODE_COUNT {
let n1 = nodes[i];
let neighbour_count = i % 8 + 3;
Expand Down
3 changes: 1 addition & 2 deletions benches/matrix_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,14 @@ fn add_5_edges_for_each_of_100_nodes(b: &mut test::Bencher) {
let edges_to_add: Vec<_> = nodes
.iter()
.enumerate()
.map(|(i, &node)| {
.flat_map(|(i, &node)| {
let edges: Vec<_> = (0..5)
.map(|j| (i + j + 1) % nodes.len())
.map(|j| (node, nodes[j]))
.collect();

edges
})
.flatten()
.collect();

b.iter(|| {
Expand Down
5 changes: 1 addition & 4 deletions src/adj.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,7 @@ pub struct EdgeReference<'a, E, Ix: IndexType> {
impl<'a, E, Ix: IndexType> Copy for EdgeReference<'a, E, Ix> {}
impl<'a, E, Ix: IndexType> Clone for EdgeReference<'a, E, Ix> {
fn clone(&self) -> Self {
EdgeReference {
id: self.id,
edge: self.edge,
}
*self
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/algo/dominators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ where
type Item = N;

fn next(&mut self) -> Option<Self::Item> {
while let Some(next) = self.iter.next() {
for next in self.iter.by_ref() {
if next.1 == &self.node {
return Some(*next.0);
}
Expand Down Expand Up @@ -267,7 +267,7 @@ where
.map(|p| *node_to_post_order_idx.get(&p).unwrap())
.collect()
})
.unwrap_or_else(Vec::new)
.unwrap_or_default()
})
.collect()
}
Expand Down
2 changes: 1 addition & 1 deletion src/algo/isomorphism.rs
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ mod matching {

// We hardcode n! values into an array that accounts for architectures
// with smaller usizes to get our upper bound.
let upper_bounds: Vec<Option<usize>> = vec![
let upper_bounds: Vec<Option<usize>> = [
1u64,
1,
2,
Expand Down
4 changes: 2 additions & 2 deletions src/algo/matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,8 @@ fn find_join<G, F>(
graph: &G,
edge: G::EdgeRef,
mate: &[Option<G::NodeId>],
label: &mut Vec<Label<G>>,
first_inner: &mut Vec<usize>,
label: &mut [Label<G>],
first_inner: &mut [usize],
mut visitor: F,
) where
G: IntoEdges + NodeIndexable + Visitable,
Expand Down
4 changes: 2 additions & 2 deletions src/algo/page_rank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ where
.enumerate()
.map(|(w, r)| {
let mut w_out_edges = graph.edges(nodeix(w));
if let Some(_) = w_out_edges.find(|e| e.target() == nodeix(v)) {
if w_out_edges.any(|e| e.target() == nodeix(v)) {
damping_factor * *r / out_degrees[w]
} else if out_degrees[w] == D::zero() {
damping_factor * *r / nb // stochastic matrix condition
Expand All @@ -90,7 +90,7 @@ where
.sum::<D>()
})
.collect::<Vec<D>>();
let sum = pi.iter().map(|score| *score).sum::<D>();
let sum = pi.iter().copied().sum::<D>();
ranks = pi.iter().map(|r| *r / sum).collect::<Vec<D>>();
}
ranks
Expand Down
2 changes: 1 addition & 1 deletion src/csr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ where
.row
.get(a.index() + 1)
.cloned()
.unwrap_or_else(|| self.column.len());
.unwrap_or(self.column.len());
index..end
}

Expand Down
11 changes: 4 additions & 7 deletions src/graph_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1733,13 +1733,10 @@ where
type Item = EdgeReference<'a, E, Ix>;

fn next(&mut self) -> Option<EdgeReference<'a, E, Ix>> {
while let Some(edge) = self.edges.next() {
if edge.node[1] == self.target_node {
return Some(edge);
}
}

None
let target_node = self.target_node;
self.edges
.by_ref()
.find(|&edge| edge.node[1] == target_node)
}
fn size_hint(&self) -> (usize, Option<usize>) {
let (_, upper) = self.edges.size_hint();
Expand Down
4 changes: 2 additions & 2 deletions src/graphmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ where

/// Add node `n` to the graph.
pub fn add_node(&mut self, n: N) -> N {
self.nodes.entry(n).or_insert(Vec::new());
self.nodes.entry(n).or_default();
n
}

Expand Down Expand Up @@ -400,7 +400,7 @@ where
} else {
exist1
};
let weight = self.edges.remove(&Self::edge_key(a, b));
let weight = self.edges.swap_remove(&Self::edge_key(a, b));
debug_assert!(exist1 == exist2 && exist1 == weight.is_some());
weight
}
Expand Down
2 changes: 1 addition & 1 deletion src/iter_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::fmt;
/// Format the iterator like a map
pub struct DebugMap<F>(pub F);

impl<'a, F, I, K, V> fmt::Debug for DebugMap<F>
impl<F, I, K, V> fmt::Debug for DebugMap<F>
where
F: Fn() -> I,
I: IntoIterator<Item = (K, V)>,
Expand Down
2 changes: 1 addition & 1 deletion tests/floyd_warshall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ fn floyd_warshall_uniform_weight() {
.iter()
.cloned()
.collect();
let res = floyd_warshall(&graph, |_| 1 as i32).unwrap();
let res = floyd_warshall(&graph, |_| 1_i32).unwrap();

let nodes = [a, b, c, d, e, f, g, h];
for node1 in &nodes {
Expand Down
8 changes: 4 additions & 4 deletions tests/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1481,7 +1481,7 @@ fn toposort_generic() {
{
order.clear();
let init_nodes = gr.node_identifiers().filter(|n| {
gr.neighbors_directed(n.clone(), Direction::Incoming)
gr.neighbors_directed(*n, Direction::Incoming)
.next()
.is_none()
});
Expand Down Expand Up @@ -1766,7 +1766,7 @@ where
G::NodeId: PartialEq,
{
// self loops count twice
let original_node = node.clone();
let original_node = node;
let mut degree = 0;
for v in g.neighbors(node) {
degree += if v == original_node { 2 } else { 1 };
Expand Down Expand Up @@ -2141,7 +2141,7 @@ fn filtered_post_order() {
Graph::from_edges(&[(0, 2), (1, 2), (0, 3), (1, 4), (2, 4), (4, 5), (3, 5)]);
// map reachable nodes
let mut dfs = Dfs::new(&gr, n(0));
while let Some(_) = dfs.next(&gr) {}
while dfs.next(&gr).is_some() {}

let map = dfs.discovered;
gr.add_edge(n(0), n(1), ());
Expand Down Expand Up @@ -2259,7 +2259,7 @@ fn test_edge_filtered() {
assert_eq!(connected_components(&positive_edges), 2);

let mut dfs = DfsPostOrder::new(&positive_edges, n(0));
while let Some(_) = dfs.next(&positive_edges) {}
while dfs.next(&positive_edges).is_some() {}

let n = n::<u32>;
for node in &[n(0), n(1), n(2)] {
Expand Down
8 changes: 4 additions & 4 deletions tests/graphmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,15 @@ fn dfs() {
{
let mut cnt = 0;
let mut dfs = Dfs::new(&gr, h);
while let Some(_) = dfs.next(&gr) {
while dfs.next(&gr).is_some() {
cnt += 1;
}
assert_eq!(cnt, 4);
}
{
let mut cnt = 0;
let mut dfs = Dfs::new(&gr, z);
while let Some(_) = dfs.next(&gr) {
while dfs.next(&gr).is_some() {
cnt += 1;
}
assert_eq!(cnt, 1);
Expand Down Expand Up @@ -250,10 +250,10 @@ fn graphmap_directed() {
// Add reverse edges -- ok!
assert!(gr.add_edge(e, d, ()).is_none());
// duplicate edge - no
assert!(!gr.add_edge(a, b, ()).is_none());
assert!(gr.add_edge(a, b, ()).is_some());

// duplicate self loop - no
assert!(!gr.add_edge(b, b, ()).is_none());
assert!(gr.add_edge(b, b, ()).is_some());
println!("{:#?}", gr);
}

Expand Down

0 comments on commit 5cabfe3

Please sign in to comment.