discuss: protocol for renaming/deprecating functions
As per initial discussion at https://github.com/igraph/rigraph/pull/1513 with @maelle and @krlmlr, I wanted to give some input / discuss the process for renaming functions.
This was triggered by the fact that as.directed (like many functions) has been renamed to snake case. However, unlike previous occasions of function renaming that I recall
- the
as_directedfunction was introduced in the same release of igraph thatas.directedwas deprecated - this was not documented in NEWS
this resulted in a warning when checking my package nat for CRAN. AFAICS the options would be:
- Immediately add
Imports: igraph (>= 2.1.0). I don't want to do this as thenatpackage is the core of a whole ecosystem and is regularly used in cluster environments where R is installed by sysdamins and often lags several years behind the current R release. Past experience suggests breakage is likely. - As I think @maelle was pointing out, I could set an option to reduce the impact of
lifecycle::deprecated_softfrom a warning to quiet/information. I didn't do this because I was nervous that the CRAN would not approve / I might not be able to replicate the test environment correctly. - To wrap the deprecated functions inside my package (commit) and call the correct function based on the installed igraph version.
As you can see I opted for 3 which required more work on my end and is somewhat inelegant but protected my end users from the consequences of 1. I would be interested to know
- if you see another option or would recommend option 2 above
- if the renaming/deprecation process on this occasion was as planned – I assume it was perhaps barring the lack of a mention in NEWS but as you can see
lifecycle::deprecated_softis not always soft ...
Finally, I just want to reiterate my sincere thanks for the fantastic work you are doing here, Greg.
Thanks for reaching out. With igraph 2.2.0 released and 2.2.1 submitted, this is a good time to discuss our stance.
- We now should have an intermediate layer between functions that we export and all autogenerated functions. (Done with AI, could have been done deterministically, but a great opportunity to build infrastructure and see limitations.)
- I want to work on a deterministic way to ensure that all
_impl()function calls only use named arguments. (AI can do that too, it's just too slow and a bit expensive.) - We're still using
.Call()in quite a few places, this should be turned into using only autogenerated functions. (AI can do that better than a human if guided, #1058.)
Once this is in place, we're in full control of the interface that we expose, irrespective of changes in the underlying library.
- New functions can be exposed easily
- If the signature of a currently exported function changes, we can provide back-compatibility for our users using the lifecycle package
- We could discuss adding an ellipsis to all functions to ensure all arguments are named.
Irrespective of what we do in this package, option 3 is a good idea for you: define a thin layer around your dependencies to shield yourself from the consequences of changes, intended or unintended. It's also more work, yes.
The library is large, we'll be relying on AI heavily to manage the sheer amount of functionality. Apologies if this causes downstream breakage, we're working towards stability while maintaining paths for evolution.
Thanks for the commentary @krlmlr. Noted and as I said previously, my continued thanks for the work you folks are doing here. Best, Greg.