igraph / rigraph

igraph R package
https://r.igraph.org
543 stars 201 forks source link

estimate_betweenness() should be deprecated #844

Open szhorvat opened 1 year ago

szhorvat commented 1 year ago

TODOS

estimate_betweenness() should be deprecated and should not appear in the docs. "Estimate" is a bit of a misnomer here as proper betweenness estimation takes a lot more than what this function does. The replacement is using the cutoff parameter of betweenness().

The same applies to estimate_closeness(), but that function already doesn't appear in the docs.

Do you have time for this @maelle ?

maelle commented 1 year ago

What about estimate_edge_betweenness()?

szhorvat commented 1 year ago

The same. All the estimate_xyz() functions are replaced by a new cutoff argument in xyz(). The reason is that limiting shortest paths to a cutoff distance is not nearly enough for getting a proper estimate of the result. What these functions produce is very useful, but it's not an estimate of the cutoff-less result.

I consider this very important because many people come to igraph without much experience in network analysis. They see the name "estimate" and decide to just use this function for a faster approximate calculation. That's not what they really do. Let's avoid names that create confusion.

The "estimate" names have already been removed from the C core.

maelle commented 1 year ago

Ok! We'll have to start with a simple warning though, then an error in another version of the package.

maelle commented 1 year ago

actually, we'll have to start with a soft deprecation because some revdeps use estimate_betweenness(): https://github.com/search?q=estimate_betweenness+org%3Acran&type=code

maelle commented 1 year ago

I'll open PRs in the corresponding repos (not the cran/ ones, which are mirrors), hopefully next week.

maelle commented 1 year ago

Even edge.betweenness.estimate is used in gDefrag

maelle commented 1 year ago

I added a TODO list in the first comment.

maelle commented 1 year ago

I'll resume the PR openings next week.

maelle commented 1 year ago

I've opened PRs in all revdeps where I could, via a GitHub search, find usage of the to-be-deprecated functions.

852 is ready for review.

maelle commented 9 months ago

The PRs have been merged/integrated into revdeps but we'd need to monitor when the updated versions go to CRAN I guess.

maelle commented 8 months ago

All PRs in revdeps merged/integrated except https://github.com/FMestre1/gDefrag/pull/2#issuecomment-1893366387

maelle commented 7 months ago

even that one is merged now