fix: local (weighted) transitivity now produces a named vector

[szhorvat]

Fixes #943. This was also a learning exercise for me after your VERTEX_QTY question, @krlmlr.

[aviator-app]

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.

---

See the real-time status of this PR on the Aviator webapp.

Use the Aviator Chrome Extension to see the status of your PR within GitHub.

[krlmlr]

Thanks. What would it take to use autogenerated R code for transitivity()? Would that be more sustainable even in the short term?

[krlmlr]

How do we know that this does what it's supposed to, without a test?

[szhorvat]

What would it take to use autogenerated R code for transitivity()?

This is a messy function that calls to one of several C functions. I'm not up for figuring out how to autogenerate this one right now.

But my personal preference would be to split this into three new functions, one for global clustering, one for local (weighted or unweighted), and one for average local (weighted or unweighted).

I'll add tests ...

[krlmlr]

We don't need to autogenerate all of it, just replace the .Call() with R-level calls to our own _impl() functions (after we generate wrappers for them).

[szhorvat]

Alright, let's try that.

[szhorvat]

To be fair, I don't know if I'd have time to do this properly. It also depends on igraph/igraph#2458 and there are issues to deal with like #1058 (comment)

I would recommend merging this as-is since it does fix a complaint. How do I know it works? I tested all four affected code paths. I can add a quick test if necessary.

[krlmlr]

A test would be great, thanks. We can merge as is, but should strive to automate as much as possible moving forward.

[szhorvat]

Well of course, in the longer term this, as most other functions, should be auto-generated. I just don't want to throw away this fix in case the autogen can't be implemented before Jan 12.

[krlmlr]

Thanks, looks good!

[krlmlr]

I have a slight preference for

Suggested change

	t1 <- transitivity(g, type = "local")
	t2 <- transitivity(g, type = "barrat")
	vs <- c("a", "c")
	t3 <- transitivity(g, type = "local", vids = vs)
	t4 <- transitivity(g, type = "barrat", vids = vs)
	expect_equal(names(t1), V(g)$name)
	expect_equal(names(t2), V(g)$name)
	expect_equal(names(t3), vs)
	expect_equal(names(t4), vs)
	t1 <- transitivity(g, type = "local")
	expect_equal(names(t1), V(g)$name)
	t2 <- transitivity(g, type = "barrat")
	expect_equal(names(t2), V(g)$name)
	vs <- c("a", "c")
	t3 <- transitivity(g, type = "local", vids = vs)
	expect_equal(names(t3), vs)
	t4 <- transitivity(g, type = "barrat", vids = vs)
	expect_equal(names(t4), vs)

because this keeps input and output closer together. No need to act here.