Repository navigation
Coverage with Julia >= 1.0 - #365
PatrickHaecker wants to merge 2 commits into
Conversation
Covers reductions over empty and one-element arrays, `checked_mul` for `f == 0`, the generic `AbstractFloat` conversion of `Normed`, hashing of values that equal a `Rational` (which goes through `decompose`), `%` of a foreign `Real` type, and the deprecated `floattype` fallback. The reductions, the hashing and `%` behave the same for `Fixed` and `Normed`, so common test functions check them over many raw types and fractional bits.
`Normed{UInt8,f}` requires `1 <= f <= 8`, so after the checks for
`f <= 7` only `f == 8` remains and the final `0.0f0` could never be
returned.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #365 +/- ##
==========================================
+ Coverage 96.75% 98.87% +2.12%
==========================================
Files 7 7
Lines 800 799 -1
==========================================
+ Hits 774 790 +16
+ Misses 26 9 -17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I've added some comments, but since this involves little changes to the main code, it's okay to merge this.
As for the code coverage, see also: #360 (comment)
| f == 8 && return Float32(Int32(x.i) * 0x155) * @f32(0xc0f0fdp-40) | ||
| 0.0f0 |
There was a problem hiding this comment.
There has been discussion regarding the support range of f since before Issue #311.
Since this is a case where it is better to apply manual dispatch (or the equivalent), I have adopted the current "sentinel".
Although this loses the ability to act as a sentinel against future changes, I think it’s a reasonable change for the purpose of increasing current coverage.
Incidentally, while f is intended to be an Int (Int64 or Int32), intentionally no type restrictions have been imposed.
This allows for “hacks” like the following:
julia> Normed{UInt8, true}(5)
5.0N7ftrue| @test_skip(@test_throws MethodError floattype(MyReal)) # TODO: eliminate `@test_skipped` when depwarn is eliminated. See #177. | ||
| # meanwhile, the deprecated fallback warns and returns the type unchanged | ||
| @test (@test_logs (:warn, r"fallback definition of `floattype`") floattype(MyReal)) === MyReal |
There was a problem hiding this comment.
| @test_skip(@test_throws MethodError floattype(MyReal)) # TODO: eliminate `@test_skipped` when depwarn is eliminated. See #177. | |
| # meanwhile, the deprecated fallback warns and returns the type unchanged | |
| @test (@test_logs (:warn, r"fallback definition of `floattype`") floattype(MyReal)) === MyReal | |
| # TODO: eliminate `@test_skip` when the deprecated fallback is eliminated. See #177. | |
| # Meanwhile, verify that the fallback warns and returns the type unchanged. | |
| @test_skip(@test_throws MethodError floattype(MyReal)) | |
| @test (@test_logs (:warn, r"fallback definition of `floattype`") floattype(MyReal)) === MyReal |
This is a matter of personal preference, but I think leading with # TODO would make it clearer that these two tests are a pair.
@test_skipped is probably a typo.
| Base.:*(x::IntReal, a::AbstractFloat) = IntReal(Int(x.i * a)) | ||
| Base.isfinite(::IntReal) = true | ||
| Base.round(x::IntReal) = x | ||
| Base.unsafe_trunc(::Type{T}, x::IntReal) where {T <: Integer} = x.i % T |
There was a problem hiding this comment.
Since this is for testing (or, more specifically, for code coverage), I suppose it can't be helped, but is it realistic to expect that unsafe_trunc is implemented for custom types?
|
|
||
| # the generic method serves `AbstractFloat` types without a dedicated method | ||
| for Tf in (Float16, Float32, Float64), x in (0.2N0f8, 0.2N0f16, 0.2N2f14, 0.2N0f32, 0.2N0f64) | ||
| @test invoke(Tf, Tuple{Normed}, x) === Tf(x) |
There was a problem hiding this comment.
Is this invoke actually called in real codes?
IIUC, in the case of a non-concrete type, dynamic dispatch should call the respective specialized methods.
I think it would probably be better to prepare a Unsigned or AbstractFloat type for testing purposes.
Also, although this is outside the scope of this PR, it might be a good idea to include BFloat16 in the test cases.
2 participants
This is the (hopefully) uncontroversial part of #350, i.e. all changes which close coverage gaps while not bumping the dependency of the Julia version.
#350 then sits on top of this PR and will become smaller once the current PR is merged.