Split adhoc arithmetic operations for MatrixElem#2419
Conversation
lgoettgens
added
the
release notes: not needed
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2419 +/- ##
=======================================
Coverage 88.12% 88.13%
=======================================
Files 130 130
Lines 32955 32983 +28
=======================================
+ Hits 29043 29069 +26
- Misses 3912 3914 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I am a bit confused by the failures. Will investigate tomorrow |
|
This uncovered a genuine bug in Nemo: Nemocas/Nemo.jl#2309 |
|
|
||
| ==(x::MatRingElem{T}, y::MatRingElem{T}) where {T <: NCRingElement} = matrix(x) == matrix(y) | ||
|
|
||
| ############################################################################### |
There was a problem hiding this comment.
I'd like to see a coherent order in the groupings of 4 functions (as though you had simply written once the block of 4 and then used cut-and-paste).
I presume the choice between NCRingElem and NCRingElement is correct.
| end | ||
|
|
||
| +(x::MatrixElem{T}, y::JuliaRingElement) where T <: NCRingElement = y + x | ||
| +(x::MatElem{T}, y::JuliaRingElement) where T <: NCRingElement = y + x |
There was a problem hiding this comment.
I'd like to see curly brackets around the where clause, as for example in line 1037 below. Personally, I believe that this improves readability...
JohnAAbbott
left a comment
JohnAAbbott
left a comment
There was a problem hiding this comment.
OK. I have made two comments about style.
3 participants
This does not change any functionality, but fixed some ambiguity issues that popped up when I played around with #2280. As this is eventually needed anyway, and does not do any breaking or conflicting changes, I just opened it as a PR right away.