Adding Option to change parallel edge behavior in adjacency_matrix functions

[danielleodigie]

• Aims to resolve Add option to adjacency matrix functions to adjust parallel edge behavior #413
• This pull request adds the ability to change the behavior of parallel edges when creating the adjacency matrix.
• Previously, these functions resorted to summing the weights of the edges
• Now, while the functions still default to summing the weights, one can choose to take the minimum, maximum, or average of the weights

[coveralls]

Pull Request Test Coverage Report for Build 5324125633

• 59 of 59 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 96.468%

---

Totals
Change from base Build 5323946941:	0.01%
Covered Lines:	15186
Relevant Lines:	15742

---

💛 - Coveralls

[raynelfss]

Everything seems to be working well. I just have a couple of code suggestions. But outside of that, LGTM!

[raynelfss]

This is good, but have you considered using a match statement instead of the nested else if clauses? It would look somewhat like this.

Suggested change

	} else if parallel_edge == "sum" {
	matrix[[i, j]] += edge_weight;
	} else if parallel_edge == "min" {
	let weight_min = matrix[[i, j]].min(edge_weight);
	matrix[[i, j]] = weight_min;
	} else if parallel_edge == "max" {
	let weight_max = matrix[[i, j]].max(edge_weight);
	matrix[[i, j]] = weight_max;
	} else if parallel_edge == "avg" {
	if parallel_edge_count.contains_key(&[i, j]) {
	matrix[[i, j]] = (matrix[[i, j]] * parallel_edge_count[&[i, j]] as f64
	+ edge_weight)
	/ ((parallel_edge_count[&[i, j]] + 1) as f64);
	*parallel_edge_count.get_mut(&[i, j]).unwrap() += 1;
	} else {
	parallel_edge_count.insert([i, j], 2);
	matrix[[i, j]] = (matrix[[i, j]] + edge_weight) / 2.0;
	}
	} else {
	return Err(PyValueError::new_err("Parallel edges can currently only be dealt with using \"sum\", \"min\", \"max\", or \"avg\"."));
	}
	}
	} else {
	match parallel_edge {
	"sum" => {
	matrix[[i, j]] += edge_weight;
	}
	"min" => {
	let weight_min = matrix[[i, j]].min(edge_weight);
	matrix[[i, j]] = weight_min;
	}
	"max" => {
	let weight_max = matrix[[i, j]].max(edge_weight);
	matrix[[i, j]] = weight_max;
	}
	"avg" => {
	if parallel_edge_count.contains_key(&[i, j]) {
	matrix[[i, j]] = (matrix[[i, j]] * parallel_edge_count[&[i, j]] as f64
	+ edge_weight)
	/ ((parallel_edge_count[&[i, j]] + 1) as f64);
	*parallel_edge_count.get_mut(&[i, j]).unwrap() += 1;
	} else {
	parallel_edge_count.insert([i, j], 2);
	matrix[[i, j]] = (matrix[[i, j]] + edge_weight) / 2.0;
	}
	}
	_ => {
	return Err(PyValueError::new_err("Parallel edges can currently only be dealt with using \"sum\", \"min\", \"max\", or \"avg\"."));
	}
	}
	}
	}

[raynelfss]

You could also use match here:

Suggested change

	} else if parallel_edge == "sum" {
	matrix[[i, j]] += edge_weight;
	matrix[[j, i]] += edge_weight;
	} else if parallel_edge == "min" {
	let weight_min = matrix[[i, j]].min(edge_weight);
	matrix[[i, j]] = weight_min;
	matrix[[j, i]] = weight_min;
	} else if parallel_edge == "max" {
	let weight_max = matrix[[i, j]].max(edge_weight);
	matrix[[i, j]] = weight_max;
	matrix[[j, i]] = weight_max;
	} else if parallel_edge == "avg" {
	if parallel_edge_count.contains_key(&[i, j]) {
	matrix[[i, j]] = (matrix[[i, j]] * parallel_edge_count[&[i, j]] as f64
	+ edge_weight)
	/ ((parallel_edge_count[&[i, j]] + 1) as f64);
	matrix[[j, i]] = (matrix[[j, i]] * parallel_edge_count[&[i, j]] as f64
	+ edge_weight)
	/ ((parallel_edge_count[&[i, j]] + 1) as f64);
	*parallel_edge_count.get_mut(&[i, j]).unwrap() += 1;
	} else {
	parallel_edge_count.insert([i, j], 2);
	matrix[[i, j]] = (matrix[[i, j]] + edge_weight) / 2.0;
	matrix[[j, i]] = (matrix[[j, i]] + edge_weight) / 2.0;
	}
	} else {
	return Err(PyValueError::new_err("Parallel edges can currently only be dealt with using \"sum\", \"min\", \"max\", or \"avg\"."));
	}
	}
	} else {
	match parallel_edge {
	"sum" => {
	matrix[[i, j]] += edge_weight;
	matrix[[j, i]] += edge_weight;
	}
	"min" => {
	let weight_min = matrix[[i, j]].min(edge_weight);
	matrix[[i, j]] = weight_min;
	matrix[[j, i]] = weight_min;
	}
	"max" => {
	let weight_max = matrix[[i, j]].max(edge_weight);
	matrix[[i, j]] = weight_max;
	matrix[[j, i]] = weight_max;
	}
	"avg" => {
	if parallel_edge_count.contains_key(&[i, j]) {
	matrix[[i, j]] = (matrix[[i, j]] * parallel_edge_count[&[i, j]] as f64
	+ edge_weight)
	/ ((parallel_edge_count[&[i, j]] + 1) as f64);
	matrix[[j, i]] = (matrix[[j, i]] * parallel_edge_count[&[i, j]] as f64
	+ edge_weight)
	/ ((parallel_edge_count[&[i, j]] + 1) as f64);
	*parallel_edge_count.get_mut(&[i, j]).unwrap() += 1;
	} else {
	parallel_edge_count.insert([i, j], 2);
	matrix[[i, j]] = (matrix[[i, j]] + edge_weight) / 2.0;
	matrix[[j, i]] = (matrix[[j, i]] + edge_weight) / 2.0;
	}
	}
	_ => {
	return Err(PyValueError::new_err("Parallel edges can currently only be dealt with using \"sum\", \"min\", \"max\", or \"avg\"."));
	}
	}
	}
	}

[enavarro51]

Thanks for working on this. You should also add 2 tests, 1 for graph and 1 for digraph, to assert that each of the options works correctly.

[enavarro51]

LGTM. Thanks for working on this.