Add missing Base methods - #82
PatrickHaecker wants to merge 7 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #82 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 6 6
Lines 272 301 +29
=========================================
+ Hits 272 301 +29 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
7a439dd to
37c9c4a
Compare
|
Please do your PRs based on the master branch: it's very hard to see whats PR specific when the PR includes other PRs like the |
37c9c4a to
b4e7c2c
Compare
Fair. I try to avoid it, but if a PR depends on changes in another PR I prefer it over doing a complicated rebase and two things being developed and tested separately which will non-trivially interact with each other. I try to always state which commits are relevant and doing the review via the "Commits" tab works very well for me, because I try to cluster the changes around suitable commits either way. Anyway, feel free to ignore PRs which build on top of other PRs until these other PRs are merged. It's just that I want to work on them while I have the problem and the code base in my mind (I guess context window is the term today) and then publish them to not forget about them. The additional advantage is that others can already use it and that they see that they don't need to work on a solution, because there already is one. |
b4e7c2c to
c0eb0f2
Compare
|
After #81 has landed and after the corresponding rebase, this PR should also be ready. Test coverage is now again at 100%. I'll keep it at draft state to indicate that the rebase is missing. |
|
Sorry, I have just realized that #68 already contains some of the changes done here. I try to harmonize them, but I am not yet sure what the best way is. |
569bc3a to
f13c3eb
Compare
All four were `MethodError`s for an angle that is not a multiple of π: negation existed only for an integer factor, and the other three not at all. Negation and conjugation rotate and reflect the angle, reduced so that both stay involutions; `abs` is `∞` whichever way the infinity points; `sign` is the unit vector, `cispi` giving it exactly on the axes. The integer factor keeps its own methods, which already returned an `Int` and are what `AllRealInfinities` relies on.
`ComplexInfinity` stores its direction in half turns, but `toinf` filled the field with the radians of `angle(x)`. The direction of an infinite complex summand was therefore off by a factor of π: `angle(toinf(complex(0, Inf)))` gave 4.93 rather than `π/2`. `_infadd` compares those angles, so a sum whose parts point the same way threw although `==` called them equal: both `complex(-Inf, 0.0) + -∞` and `complex(0.0, Inf) + im*∞` raised an ArgumentError. Only the positive real axis escaped, angle `0` being the fixed point of the missing scaling. `_sb` is the conversion the multiplication already uses, so it moves above the addition and both sections share it.
Both were `MethodError`s, which also made `∞ in 1:5` fail, a range asking `isinteger` before it compares. `Inf` is not an integer and rounding leaves it alone, so an infinity does the same and returns itself. `InfiniteCardinal` is left out of both: it is an `Integer`, for which `Base` already returns `true` and the value unchanged.
`∞ / 2` and `2 / ∞` were promotion errors, though `inv` was already there to build them from: division is multiplication by the inverse, which brings the sign and the `NaN` handling of `*` with it. `\` needs nothing of its own, `Base` defining it as `y / x`. `∞ / ∞` returns `NotANumber`, as `div(∞, ∞)` and `mod(∞, ∞)` already do, rather than the `NaN` of the floats. `2 / ∞` inherits the `Int` zero of `inv(∞)` where the floats give `0.0`, which fixing `inv` will settle in one place. `Rational` and `Complex` need the same explicit pairs in `ambiguities.jl` as the other operators.
`3 % ∞` was a promotion error, though `mod` and `div` were both already there. `rem` keeps the sign of the dividend, so unlike `mod` it needs no bound: `-3 % ∞` is `-3`, where `mod(-3, ∞)` is unbounded and throws. `divrem` follows from the two. The other direction returns `NotANumber`, as `mod(∞, x)` and `div(∞, ∞)` do. `Rational` and `BigInt` need the explicit pairs in `ambiguities.jl`, an `InfiniteCardinal` being an `Integer` that `Base` has its own methods for.
`∞ ≈ Inf` threw, `Base` promoting its arguments before it compares them and an infinity having no common type with a number. Nothing is near an infinity but an equal one, which is what the floats do too, so approximate equality is exact equality and the keywords have nothing to loosen.
Gaps that JuliaMath#68 covers and this branch did not, plus the types they missed. `round(x, ::RoundingMode)` was a MethodError, and `round(x; digits)` fell through to `Base` and returned `Inf` rather than the infinity, disagreeing with the plain `round(x)` next to it. `isinteger` and the four rounding functions were also defined for `Infinity` and `RealInfinity` alone, so a `ComplexInfinity` raised a MethodError where `Base` returns `false` and the value itself for the matching `Complex`, and `ℵ₀` took a rounding mode but not the keywords. `float(::ComplexInfinity)` was a MethodError, the real infinities having got theirs from the `AbstractFloat` conversion. JuliaMath#68 proposes `exp(im*angle(x))*Inf`, which is unsound: `0 * Inf` is a `NaN`, so `float(ComplexInfinity())` gives `Inf + NaN*im`, and the imaginary and negative real axes come back as diagonals. `cospi`/`sinpi` are exact at the half-integers, so building the parts from them keeps the axes exact. Two saturating parts can express only eight rays, so an angle off them lands on the nearest one, which the test pins.
f13c3eb to
07eff97
Compare
2 participants
This PR builds on #81, so only the last six commits belong to this PR. Once #81 lands we can rebase this ontoI recommend reviewing commit by commit.masterto shrink the diff.Everything here is purely additive: each method below is a
MethodErrortoday.ComplexInfinitygainsabs,sign,conjand negation, where the<:Integermethods exist because the generic angle arithmetic would widenComplexInfinity{Bool}toComplexInfinity{Int64}.isintegerplusround,floor,ceilandtrunc, which also fixes∞ ∉ 1:5because a range asksisintegerbefore it compares./,\and//, of which\needs no method of its own becauseBasedefines it asy / x.rem,%anddivrem, where the threedivremmethods are needed becauseBase'sInteger-onlydivremcomputesa - div(a,b)*binstead of callingrem, which anInfiniteCardinalcannot evaluate.isapprox, becauseBasecompares only after promoting, which fails for an infinity and a number.The tests check an infinite tolerance against the float tolerance for every pair from a value list, skipping the one case where
Baseitself is wrong:isapprox(0, 0; rtol=Inf)isfalsebecause itsIntegermethod omits thex == yshort-circuit and so evaluatesmax(0, Inf*0).