-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 ^(x, p::Integer)
#23332
Deprecate ^(x, p::Integer)
#23332
Conversation
c3967af
to
86a5d91
Compare
Fun fact: this method predates the announcement of Julia |
I don't think this is the right fix, to make integer powers work for your own type you only need to overload julia> struct Foo x end
julia> Base.:*(x::Foo, y::Foo) = Foo(x.x*y.x)
julia> x = Foo(2)
Foo(2)
julia> x^3
WARNING: x ^ p::Integer is deprecated, use power_by_squaring(x, p) instead.
Stacktrace:
[1] depwarn at ./deprecated.jl:68 [inlined]
[2] ^(::Foo, ::Int64) at ./deprecated.jl:56
[3] literal_pow(::Base.#^, ::Foo, ::Val{3}) at ./intfuncs.jl:214
[4] eval(::Module, ::Expr) at ./repl/REPL.jl:3
[5] eval_user_input(::Any, ::Base.REPL.REPLBackend) at ./repl/REPL.jl:69
[6] macro expansion at ./repl/REPL.jl:100 [inlined]
[7] (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:96
while loading no file, in expression starting on line 0
Foo(8) |
Is there a case where you would do that and your type is not a |
Your type has to implement |
I don't know, but I am sure someone does it :) In general, it is nice that you don't have to overload everything to get things to work; if you define |
Alternate proposal: ^(x, p) = power_by_squaring(x, p) Then you could freely override |
power_by_squaring isn't valid for non-integer powers though |
If you define ^(::MyType, x::Real) = ...
^(::MyType, x::Integer) = ... everything works, right? Probably these methods are different anyways since you can do some faster stuff in the Integer case. |
@fredrikekre No, those methods are not different in this case. @tkelman You would get a method error on |
^(x::MyType, p::Real) = mypow(x, p)
^(x::MyType, p::Integer) = mypow(x, p)
mypow(x::MyType, p::Real) = ... I just worry that since this is such a general method that is deprecated, it will essentially look like |
It definitely isn't that simple in our case: https://github.com/invenia/Nabla.jl/blob/master/src/sensitivities/scalar.jl |
It would lead at least some people to think they should be extending |
Triage accepts. |
@alanedelman just complained to me about this because he likes to do classroom demos where he defines |
Well, there's always the option of defining |
This caused method ambiguities when defining
^(::MyType, ::Real)
which I think is reasonably common when overloading^
.I ran tests I thought were related and got no failures; we'll see what CI says about the full suite.
power_by_squaring
makes a bunch ofNumber
-related assumptions so I doubt anyone was using this in the wild.