Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

estimate_betweenness() should be deprecated #844

Closed
1 of 4 tasks
szhorvat opened this issue Jun 15, 2023 · 13 comments
Closed
1 of 4 tasks

estimate_betweenness() should be deprecated #844

szhorvat opened this issue Jun 15, 2023 · 13 comments
Assignees
Labels
documentation lifecycle Deprecating old APIs
Milestone

Comments

@szhorvat
Copy link
Member

szhorvat commented Jun 15, 2023

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 ?

@szhorvat szhorvat added this to the 1.5.0 milestone Jun 15, 2023
@maelle maelle added documentation lifecycle Deprecating old APIs labels Jun 16, 2023
@maelle maelle self-assigned this Jun 16, 2023
@maelle
Copy link
Contributor

maelle commented Jun 16, 2023

What about estimate_edge_betweenness()?

@szhorvat
Copy link
Member Author

szhorvat commented Jun 16, 2023

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
Copy link
Contributor

maelle commented Jun 16, 2023

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

@maelle
Copy link
Contributor

maelle commented Jun 16, 2023

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
Copy link
Contributor

maelle commented Jun 16, 2023

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

@maelle
Copy link
Contributor

maelle commented Jun 16, 2023

Even edge.betweenness.estimate is used in gDefrag

@maelle
Copy link
Contributor

maelle commented Jun 16, 2023

I added a TODO list in the first comment.

@maelle
Copy link
Contributor

maelle commented Jun 16, 2023

I'll resume the PR openings next week.

@maelle
Copy link
Contributor

maelle commented Jun 19, 2023

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.

@krlmlr krlmlr modified the milestones: 1.5.0, 2.0 Jun 20, 2023
@maelle
Copy link
Contributor

maelle commented Dec 4, 2023

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

@maelle
Copy link
Contributor

maelle commented Jan 16, 2024

All PRs in revdeps merged/integrated except FMestre1/gDefrag#2 (comment)

@maelle
Copy link
Contributor

maelle commented Feb 9, 2024

even that one is merged now

@maelle
Copy link
Contributor

maelle commented Oct 15, 2024

I'll close this: since we use lifecycle for this deprecation, it'll be looked at when we increase deprecation levels anyway.

@maelle maelle closed this as completed Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation lifecycle Deprecating old APIs
Projects
None yet
Development

No branches or pull requests

3 participants