get_clusters fix

[finlaycampbell]

See #107

[zkamvar]

@finlaycampbell, can you explain what's happening here and maybe write a test for this?

[finlaycampbell]

The output from igraph in the command below was coercing case IDs to character:

epicontacts/R/get_clusters.R

Lines 90 to 93 in 7f5f401

	net_nodes <- setNames(data.frame(id =igraph::V(net)$id,
	cluster_member = cs$membership,
	stringsAsFactors = FALSE),
	c("id", member_col))

This caused an error when trying to full_join this with linelist case IDs, if these were not characters. The fix coerces the igraph output IDs to whatever class the linelist case IDs are. I've added some tests that try get_clusters on IDs with different classes.

[zkamvar]

The output from igraph in the command below was coercing case IDs to character:

epicontacts/R/get_clusters.R

Lines 90 to 93 in 7f5f401

	net_nodes <- setNames(data.frame(id =igraph::V(net)$id,
	cluster_member = cs$membership,
	stringsAsFactors = FALSE),
	c("id", member_col))

This caused an error when trying to full_join this with linelist case IDs, if these were not characters. The fix coerces the igraph output IDs to whatever class the linelist case IDs are. I've added some tests that try get_clusters on IDs with different classes.

Can you add a test that checks if the input is a factor?

[codecov]

Codecov Report

Merging #108 into master will increase coverage by 0.17%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #108      +/-   ##
==========================================
+ Coverage   83.76%   83.93%   +0.17%     
==========================================
  Files          19       19              
  Lines         659      660       +1     
==========================================
+ Hits          552      554       +2     
+ Misses        107      106       -1

Impacted Files	Coverage Δ
R/get_clusters.R	100% <100%> (ø)	⬆️
R/make_epicontacts.R	95.74% <0%> (+2.12%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f5f401...992350f. Read the comment docs.

[finlaycampbell]

Done

[zkamvar]

This is failing on push, which might mean that this fix is affecting the new tests added in 7f5f401. Try merging master into this branch and see how the tests fare.

[zkamvar]

Either that, or it means that the tests are failing on this branch because the tests were failing earlier. The PR/push checks really are the USB-socket of checks.

[finlaycampbell]

Ah didn't realise I hadn't merged the master branch fixes - hopefully working now. I also had to update one of the reference rds files because it seemed to have been created using an older version of outbreaks.

[zkamvar]

Ah didn't realise I hadn't merged the master branch fixes - hopefully working now. I also had to update one of the reference rds files because it seemed to have been created using an older version of outbreaks.

In general, I'm against changing these tests because it's difficult to asses exactly what has changed, but IIRC, that particular test doesn't actually run anywhere but on a local machine (see #88), so it's all good.

[finlaycampbell]

In general, I'm against changing these tests because it's difficult to asses exactly what has changed, but IIRC, that particular test doesn't actually run anywhere but on a local machine (see #88), so it's all good.

Yeah agreed - generally I'm not sure how good of an idea referencing saved objects is.

[zkamvar]

Yeah agreed - generally I'm not sure how good of an idea referencing saved objects is.

[narrator] It wasn't a good idea.

[zkamvar]

LGTM