Deprecate PauliBasis and equal_bases
#39
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #39 +/- ##
===========================================
+ Coverage 17.08% 32.66% +15.58%
===========================================
Files 12 13 +1
Lines 398 401 +3
===========================================
+ Hits 68 131 +63
+ Misses 330 270 -60 ☔ View full report in Codecov by Sentry. |
|
I am leaning more towards a deprecation for renaming, not for complete removal. Currently PauliBasis does provide something that is otherwise not available: an implicit way to define a multiqubit basis in O(1) space. That is a valuable alternative to the O(n) way of defining the same with SpinBasis(1//2). At some point in the future, when the necessary capabilities exist we can just silently remove the struct and keep the constructor, returning the appropriate composition of SpinBasis(1//2). The principle here being: if something was added, but it is inelegant, do not remove it, just sequester it so that it does not cause future issues. That way old code will still work -- we should not break old code purely for the sake of elegance. |
|
Maybe we can call it |
src/bases.jl
Outdated
| write(stream, "Fock(cutoff=$(x.N), offset=$(x.offset))") | ||
| Base.@deprecate equal_bases(a, b) _equal_bases(a, b) false | ||
| function _equal_bases(a, b) | ||
| Base.depwarn("`equal_bases` will be removed in next version!", :_equal_bases) |
There was a problem hiding this comment.
Similarly here, do not say it will be removed, rather say what should be used instead. This will probably remain in the code base for many years to avoid breaking old code. We can just steer future users to not use it.
There was a problem hiding this comment.
If you use @deprecated, I do not think you need to manually run depwarn.
julia> @macroexpand1 Base.@deprecate f(x) g(x)
:($(Expr(:toplevel, :(export f), :(f(x) = begin
#= deprecated.jl:103 =#
$(Expr(:meta, :noinline))
#= deprecated.jl:104 =#
Base.depwarn("`f(x)` is deprecated, use `g(x)` instead.", ((Base.Core).Typeof(f)).name.mt.name)
#= deprecated.jl:105 =#
g(x)
end))))
There was a problem hiding this comment.
Got it! In this instance, I think the depwarn is more useful than @depricate since then the appropriate message of what should be used instead will be printed.
I renamed to |
akirakyle
force-pushed
the
deprecate
branch
3 times, most recently
from
0998f71 to
4b05311
Compare
|
@Krastanov sorry for all the force pushes on this, I kept adding things to the cleanup commit. I think it's best just to review/merge that since it doesn't change any functionality. I've changed this one to draft since I'm still actually struggling with #35 and there's a chance it might be best to actually consider the actual Pauli operator basis as a subtype of |
|
Closing for now, see #40 for why. |
|
Reopening, since the actual Pauli operator basis will have a different structure than the currently named |
Closes #36, applies on top of #37