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

Deprecate, then remove mode="total" #914

Closed
szhorvat opened this issue Oct 23, 2023 · 4 comments
Closed

Deprecate, then remove mode="total" #914

szhorvat opened this issue Oct 23, 2023 · 4 comments
Assignees
Labels
lifecycle Deprecating old APIs

Comments

@szhorvat
Copy link
Member

szhorvat commented Oct 23, 2023

The "neighbour mode" mode parameter (igraph_neimode_t in C) can take values "out", "in", "all" and "total". However, "total" is just an alias for "all", and in my opinion in most contexts it is not a fitting name. The naming perhaps makes sense for degrees, but not for situation where it described in which direction to follow edges.

The same name has already been removed from the C core.

Can we start a deprecation process for R as well? Is there a clean deprecation route that is compatible with the auto-generation we are using, @maelle ?

@krlmlr At some point can you remove "total" from auto-generation and run revdepchecks, to see who is using it?

@szhorvat szhorvat added this to the upgrade milestone Oct 23, 2023
@szhorvat
Copy link
Member Author

Many functions which are not auto-generated already do not use "total". Consistency is another reason to remove it. Perhaps we can keep it for degree and strength only, if revdepchecks indicate that people use it there.

@maelle maelle self-assigned this Nov 10, 2023
@szhorvat
Copy link
Member Author

szhorvat commented Nov 21, 2023

This is very low priority. Given the workload we have, I would put this off indefinitely. I suggest not to spend time on it now @maelle.

@krlmlr krlmlr removed this from the upgrade milestone Dec 29, 2023
@maelle maelle added the lifecycle Deprecating old APIs label Jan 15, 2024
@maelle
Copy link
Contributor

maelle commented Dec 10, 2024

Low priority, but it wouldn't be hard to put this into the lifecycle system, so please ping me again if you change your mind @szhorvat

@szhorvat
Copy link
Member Author

Given how constrained we are for resources, I suggest dropping this task.

"total", as a descriptor, is a perfect fit for degree and strength, and not so good for all the rest. But some people may still be using it, so removing it is a breaking change.

Doing this would in the end be quite a bit of work for just an aesthetic improvements. I think we have higher priorities, such as exposing new functionality ...

@maelle maelle closed this as not planned Won't fix, can't repro, duplicate, stale Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle Deprecating old APIs
Projects
None yet
Development

No branches or pull requests

3 participants