-
Notifications
You must be signed in to change notification settings - Fork 58
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
Yet more flinitfy (and related tweaks) #1876
Conversation
Also remove redundant `jacobi_symbol` method.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1876 +/- ##
==========================================
- Coverage 87.34% 87.30% -0.04%
==========================================
Files 97 97
Lines 35832 35745 -87
==========================================
- Hits 31297 31207 -90
- Misses 4535 4538 +3 ☔ View full report in Codecov by Sentry. |
@@ -396,11 +396,11 @@ function -(a::QQFieldElem, b::AbsSimpleNumFieldElem) | |||
return r | |||
end | |||
|
|||
+(a::AbsSimpleNumFieldElem, b::Integer) = a + ZZRingElem(b) | |||
+(a::AbsSimpleNumFieldElem, b::Integer) = a + flintify(b) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test coverage for adhoc operations is really bad. Would be good to have more of those in the interface tests (possibly optionally, as I am not sure we mandate this in the ring interface?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the ring interface states that it is optional to add explicit ad-hoc functions, but AA should take care of the fallback using promotion (see https://nemocas.github.io/AbstractAlgebra.jl/dev/ring_interface/#Optional-binary-ad-hoc-operators)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the matrix constructor changes are very hard to follow. maybe the next time put moving and changing into two different commit? Anyway, I don't see anything bad standing out
Also remove redundant
jacobi_symbol
method.