Replace type declarations #2981
Conversation
Type declarations insert `convert` calls. Instead, use type assertions (which just verify the object has the right type to start with), or fix the code to actually be type stable and return the right type.
| end | ||
| n = AbstractAlgebra.leading_coefficient(top_form.f) | ||
| m = AbstractAlgebra.leading_coefficient(polynomial(volume_form(toric_variety(c))).f) | ||
| return QQFieldElem(n//m) | ||
| return QQFieldElem(n,m) |
There was a problem hiding this comment.
@fingolfin Unless I am mistaken, this change leads to the CI-failures in 1.6 ubuntu-latest: no method matching QQFieldElem(::QQFieldElem, ::QQFieldElem).
Generally speaking, your changes look good to me. In particular, I would hope that once the above line is fixed, the tests pass again.
There was a problem hiding this comment.
Maybe QQFieldElem(ZZ(n), ZZ(m)) might work.
There was a problem hiding this comment.
I just tested this fix, and it seems to work. So I have taken the liberty to push a corresponding fix.
(Origin: m, n are coefficients of polynomials with rational coefficients. So by construction, they are rational numbers.)
There was a problem hiding this comment.
Modulo the minor change above, this PR looks good to me. Thank you @fingolfin . (But as always, let us see if the tests pass...)
|
Tests are not passing with a genuine error |
src/AlgebraicGeometry/ToricVarieties/CohomologyClasses/methods.jl
Outdated
Show resolved
Hide resolved
Co-authored-by: Tommy Hofmann <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2981 +/- ##
==========================================
- Coverage 80.22% 79.88% -0.35%
==========================================
Files 473 473
Lines 67257 67909 +652
==========================================
+ Hits 53960 54249 +289
- Misses 13297 13660 +363
|
Type declarations insert
convertcalls. Instead, use type assertions(which just verify the object has the right type to start with), or
fix the code to actually be type stable and return the right type.