Add conformance tests for some adhoc operations #1867
Conversation
|
Nice, this found already a genuine bug in AA (see second commit) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1867 +/- ##
=======================================
Coverage 88.13% 88.13%
=======================================
Files 120 120
Lines 30282 30286 +4
=======================================
+ Hits 26689 26694 +5
+ Misses 3593 3592 -1 ☔ View full report in Codecov by Sentry. |
|
Seems to be passing locally now but reveals issues in Nemo (and/or Nemo reveals issues in the tests, dunno 😂 ) |
fingolfin
force-pushed
the
mh/conformance-test-adhoc
branch
from
22d75a9
to
60b39e0
Compare
The following lead to a crash (uncovered by Nemocas/AbstractAlgebra.jl#1867) ```julia using Nemo R, t = ZZ[:t] zero(R) - ZZ(0) ``` This is due to a bug in FLINT's `fmpz_poly_sub_fmpz`.
The following lead to a crash (uncovered by Nemocas/AbstractAlgebra.jl#1867) ```julia using Nemo R, t = ZZ[:t] zero(R) - ZZ(0) ``` This is due to a bug in FLINT's `fmpz_poly_sub_fmpz`.
The following lead to a crash (uncovered by Nemocas/AbstractAlgebra.jl#1867) ```julia using Nemo R, t = ZZ[:t] zero(R) - ZZ(0) ``` This is due to a bug in FLINT's `fmpz_poly_sub_fmpz`.
The following lead to a crash (uncovered by Nemocas/AbstractAlgebra.jl#1867) ```julia using Nemo R, t = ZZ[:t] zero(R) - ZZ(0) ``` This is due to a bug in FLINT's `fmpz_poly_sub_fmpz`.
src/generic/UnivPoly.jl
Outdated
| @@ -516,9 +516,15 @@ for op in (:+, :-, :*) | |||
| return UnivPoly{T}($op(data(p),n), S) | |||
| end | |||
|
|
|||
| $op(n::Union{Integer, Rational, AbstractFloat}, p::UnivPoly) = $op(p,n) | |||
| function $op(n::Union{Integer, Rational, AbstractFloat}, p::UnivPoly{T}) where {T} | |||
There was a problem hiding this comment.
Urgh, the UnivPoly adhoc subtraction was wrong since my recent PR #1877. Luckily the tests here caught it.
|
This of course fails the |
|
(But it is also OK for me to punt this a bit longer, it's not that urgent. But I look forward to the increased code coverage) |
|
I am fine with putting this in a patch release (after I had a chance to look at it, let's put it on my to-do list). |
src/julia/Integer.jl
Outdated
| print(io, "Integers") | ||
| end | ||
|
|
||
| function show(io::IO, R::Integers{T}) where T | ||
| print(io, "Integers{$T}()") | ||
| end |
There was a problem hiding this comment.
can you add a similar show change for Rationals and Floats as well? I think it would be great to have these three as aligned as possible
There was a problem hiding this comment.
I have removed the printing tweak from this PR and put it into a separate PR. It can adjusted there (or not) as people have time, without holding up this useful PR (including the bug fix in it) further
fingolfin
force-pushed
the
mh/conformance-test-adhoc
branch
from
2fc343b
to
8d6746d
Compare
Work in progress towards #1817...