I’m preparing a new version of WringTwistree because I found a bug. Some functions are exported only because they’re used in tests; when I wrote it, I hadn’t figured out how to access an unexported function. (I previously wrote it in Haskell, in which you can’t access an unexported function.) So I have to deprecate them. @deprecate takes the function to deprecate and the function to use instead. For encryptPar! and friends, I can use encrypt! instead, with :parallel as the third argument. But cycleRotBitcount has no purpose for the end user and shouldn’t be part of the API at all. How do I say that cycleRotBitcount is deprecated, without saying what to use instead?
2 Likes
I just Base.depwarn but I would like to know what the best practices are now. @deprecate, a few internal undocumented macros (listed below), and other packages are built on depwarn, too.
help?> Base.@deprecate
@deprecate
@deprecate_binding
@deprecate_moved
@deprecate_stdlib
The help for @deprecate says “The warnings are printed from tests run by Pkg.test().” That’s where I don’t want them printed, as cycleRotBitcount is used for testing the internal function rotBitcount and has no other use. Should I put the deprecation in the docstring of cycleRotBitcount?
Pkg.test(;julia_args=–depwarn=no) lets you skip deprecation warnings (usually for the better performance), but that obviously has the drawback of taking away your deprecation tests. I think the least editing you could do is rename the cycleRotBitcount methods to a different internal function like _cycleRotBitcount and support the deprecated cycleRotBitcount(args...; kwargs...) = _cycleRotBitcount(args...; kwargs...). Then your test can import and use the internal _cycleRotBitcount for your tests. Obviously awkward, but you can just make a note to move that internal to the test code in a major release, if you still need it by then. Someone may have a better idea, I’ve never had to keep using a deprecated function for anything other than testing its own behavior.
You should document it explicitly in the documentation for a minor release or two, just to let users get used to it. But at some point, it being unexported, not public, and its docstring being absent from the documentation is enough.
You might consider changing its name for your internal use, and turning the old function into a wrapper for it, which also calls @warn "This function is not for external use" maxlog = 1.
IMO documentation of the change should be enough, but it’s never really enough.
EDIT - just realized that’s the same thing @Benny recommended. 
1 Like
This is not an implementation of someone else’s cipher, but one I invented and published last year, so there are probably not many users, and there are probably no users of cycleRotBitcount besides the test code, even though it had a docstring, as what it does is not useful except for testing. So I think it’s sufficient to note in the docstring that it’s not to be used except in test code.
As to encryptPar!, @deprecate encryptPar!(wring,buf) encrypt!(wring,buf,:parallel) would result in an infinite recursion, since encrypt! calls encryptPar!. So for this function too, the thing to do is put a big warning in the docstring.
@deprecate is documented to:
Deprecate method old and specify the replacement call new, defining a new
method old with the specified signature in the process.
The intention is to move your deprecated function’s methods or code to the new function, then @deprecate automatically defines some methods to keep supporting the deprecated function.
What’s happening is you wrote a new encrypt!(wring,buf,:parallel) method forwarding to the deprecated encryptPar!(wring,buf) method, then @deprecate overwrites encryptPar!(wring,buf) with encrypt!'s code to forward to itself, effectively erasing the actual implementation. While you could just depwarn encryptPar!, that means encrypt! will also print deprecation warnings if enabled. API should not use deprecated names at any level, so you’ll have to move code out of deprecated functions if you want to keep using the code elsewhere.
I’m not planning to delete the functions, I’m planning to only not export them. If that’s not deprecation, what is it? Is there a way to issue a warning when a function is called from another module, but not when it’s called from the same module or from test code?
None of the suggestions so far needed to delete any function. The issue is if you add a runtime deprecation warning (which is typical) against calling a deprecated function, then you don’t want your API ever calling the deprecated function when those deprecation warnings are enabled (which is the default in tests).
That’s a bit of an oversimplification. For one, you can deprecate public names that weren’t ever exported to begin with. For another, the only thing the deprecated name should do is support obsolete code before the next major release; the code you’re actively developing is NOT obsolete. Deprecation does not mean moving a name from the API to the internals of the remaining API; you are not an exception to the discouragement of deprecated names.
No, method bodies don’t know where you put the function call. Macros can expand to module-conditional warnings, but you’re not deprecating a macro.
So then I’m not deprecating them. What is what I’m doing called?