igraph / rigraph

igraph R package
https://r.igraph.org
542 stars 200 forks source link

Identify deprecated functions #685

Open maelle opened 1 year ago

maelle commented 1 year ago

Functions with dots in their name that are not deprecated (or there is no alternative)

(This is a community-edited section.)

maelle commented 1 year ago

related #668

szhorvat commented 1 year ago

There is also the zzz-deprecate.R files.

A a general guideline, most functions with . in their name are deprecated. But there are a few with dots which aren't, which we should collect in this post. I will edit the ones I'm aware of into the top post.

There are also a few functions which are technically deprecated (they appear in zzz-deprecate.R), but there is no alternative that provides their functionality, see e.g. #588.

maelle commented 1 year ago

So some of them are deprecated and others are superseded (there's a replacement) according to the terminology of https://lifecycle.r-lib.org/articles/stages.html

maelle commented 1 year ago

From #662 (copying justifications/replacements)

erdos.renyi.game() should not be included. We have the consistently named sample_gnm and sample_gnp that provide the exact same functionality.

maelle commented 1 year ago
maelle commented 1 year ago

Not sure why handle_vertex_type_arg has #' @keywords internal @ntamas (tagging you because of git blame :innocent: )

maelle commented 1 year ago
szhorvat commented 1 year ago
  • layout.spring() -> layout_with_fr() (yes the same as above)

  • layout.svd -> layout_with_fr() (yes the same as above a third time)

To be clear, these two were not the FR layout originally. The original methods have been removed. Once again, these names should IMO be removed for good. I will support all spring cleaning proposals :-)

But to be clear, this is not my decision.

maelle commented 1 year ago

@szhorvat is as.igraph() really deprecated?

ntamas commented 1 year ago

Hmm, maybe it's time to start a checklist? (I'll try to stick to the lifecycle terminology).

That's all that I've found in the docs so far.

szhorvat commented 1 year ago
  • layout.fruchterman.reingold.grid() is deprecated without replacement. Right now it calls layout_with_fr() instead, but this function is not a replacement.

Isn't layout_with_fr(grid='grid') equivalent? It's not a direct renaming, but the functionality is provided.


layout.spring() and layout.svd() no longer provide their original functionality, so is it reasonable to keep them? They have been issuing warnings for a while now, so people who used them should have noticed that something changed.


I completely agree on ego(), it's specific to social sciences, and not widely used outside of that field. I wanted to bring this up too.

ntamas commented 1 year ago

Re layout.spring() and layout.svd(): I was advocating for removal as well, so +1.

Re layout_with_fr(): indeed, I did not notice the grid parameter, which means that the original functionality of layout.fruchterman.reingold.grid() is indeed provided. I'll update my comment above.

maelle commented 1 year ago

With all these deprecations it might be worth also creating a special article about them? Maybe even also published as a blog post if there's infrastructure for it? Tying together the new pkgdown website with its organized reference, with the deprecation changes (so communicating it as an increase in clarity).

Should all the removals be tied to a major version change?

For each function to be removed/deprecated I suggest to look at whether they're used in reverse dependencies (so at least those aren't broken by the update).

I find this Twitter thread also relevant https://twitter.com/hadleywickham/status/1582788323206012928

In any case I won't start implementation before #660 is merged. :innocent:

ntamas commented 1 year ago

660 is closed - shall we reopen it and consider for a merge?

maelle commented 1 year ago

oops I meant #662 that I need to fix up

szhorvat commented 1 year ago

I just recalled a problematic dotted name, subgraph.edges. There is no current underscore equivalent. The docs state thst subgraph (currently equivalent to induced_subgraph) is intended to be used as the future name of this functionality.

It would be good to discuss this again.

ntamas commented 1 year ago

That was writted / decided by Gábor and I'm not really sure whether I like it. induced_subgraph() is explicit and implies that the input is a vertex set and the result will be the induced subgraph. Similarly, the corresponding edge-based function should have a name that also emphasizes that the input must be an edge set. subgraph_edges() might not be the best name, but at least it includes "edges". subgraph() would leave no clue to the user as to whether it works with vertex or edge sets as input.

szhorvat commented 1 year ago

I agree.

In the C core, we recently deprecated subgraph_edges() in favour of subgraph_from_edges(). How about using that to make the name a bit more natural?

krlmlr commented 1 year ago

as.hclust() is a generic from the stats package, we can't change that.

maelle commented 9 months ago

Maybe come back to this issue after #697 is done.

szhorvat commented 8 months ago

There are still a bunch of functions using .Deprecated:

$ rg '\\.Deprecated'
iterators.R
532:    .Deprecated(".nei")
539:    .Deprecated(".innei")
546:    .Deprecated(".outnei")
563:    .Deprecated(".inc")
567:    .Deprecated(".inc")
583:    .Deprecated(".from")
599:    .Deprecated(".to")
880:    .Deprecated(".inc")
884:    .Deprecated(".inc")
897:    .Deprecated(".from")
910:    .Deprecated(".to")

layout.R
868:  .Deprecated("layout_on_grid", msg = paste0(

basic.R
63:  .Deprecated("ends", msg = paste(
maelle commented 5 days ago

rd file called layout.deprecated