-
-
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
imag(pi)
gives an error
#31949
Comments
There exists an almost unbounded number of methods that are possible to add to Instead, it would be better to document that the |
I disagree. The number is definitely bounded and roughly equal to the number of functions in (Note that the two-argument functions don't require a square number of methods to be added. Two different Adding these methods requires less than one line of code per method (plus tests), on average. For example, all of the two-argument methods listed in #26701 can be created by making an existing line slightly longer. The problem is not that users are likely to do things like
Such tests should not fail simply because a function in |
These definitions would enable many interoperables mentioned here and elsewhere. Base.complex(x::Irrational{S}) where {S} = complex(x, 0.0)
Base.imag(x::Irrational{S}) where {S} = 0.0
Base.angle(x::Irrational{S}) where {S} = 0.0 |
Just ran into a similar issue with negative powers: julia> pi^-1
ERROR: MethodError: no method matching Irrational{:π}(::Int64) EDIT: Gah, sorry! This has been fixed, I was on 1.1.0 🤦. |
that is expected ( precedence )
|
I'm starting to get real itchy to delete |
please do -- maybe first move it to a new module "Base.<shifted space>" |
One thing that the above-mentioned
Other functions that would also be fixed by the above are: The documentation does not require One option would be to use |
Maybe we should define this instead: Base.zero(::Irrational) = false
Base.zero(::Type{<:Irrational}) = false
Base.one(::Type{<:Irrational}) = true That would reduce the amount of 64-bit poisoning that happens? |
Yes. That was what I was trying to say in the last paragraph of my previous comment. |
Two more methods that definitely need fixing (unless |
We're not going to remove |
I had guessed as much. I'd be happy to make a PR with the above-mentioned After that, there should not be many problematic functions left in Base. A naïve for loop, testing for functions that accept |
Given that, how is one supposed to construct I found this issue when I discovered that |
|
…he Julia bug JuliaLang/julia#31949. Added operations to retrieve spline control points.
We discussed this on triage, and felt that it is better to be explicit about the transforms for irrational numbers into the appropriate computational types and not add more methods to them. Just worth noting that the links in the relevant discussion were closed for the same reason in years past, so we seem to be pretty consistent here still in our thinking. |
Attempting to take the imaginary part of an
Irrational
currently gives this error:A straightforward remedy would be to define
imag(::Irrational) = 0.0
Alternatively, defining
zero(::Irrational) = 0.0
would fix this issue, and at the same time fix the issue discussed in this thread: https://discourse.julialang.org/t/complexifying-pi/23885 This is the solution I'd personally prefer. However, I am not sure whether0.0
constitutes an "additive identity" in this case, since adding it causes loss of precision.(For more missing methods, see #26701)
For relevant discussion see: #21204, #22928
The text was updated successfully, but these errors were encountered: