search

Nemocas / Nemo.jl

Julia bindings for various mathematical libraries (including flint2)
http://nemocas.github.io/Nemo.jl/
Other
187 stars 58 forks source link

Remove redundant identity_matrix methods #1782

Closed fingolfin closed 3 months ago

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.90%. Comparing base (3d975fd) to head (f74423f). Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1782 +/- ## ========================================== - Coverage 85.95% 85.90% -0.06% ========================================== Files 95 95 Lines 36503 36491 -12 ========================================== - Hits 31375 31346 -29 - Misses 5128 5145 +17 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

thofma commented 3 months ago

Can you elaborate on the "redundant"? I have not found any other call to fmpq_mat_one, so I am not sure where the redundancy comes from.

fingolfin commented 3 months ago

@thofma you are 100% right, this method (and two others using ccall) should not be removed. I removed the first couple which are essentially equivalent to the default implementation, and then didn't pay attention. I guess I should stop making PRs at 01:00 :-(, at the very least those should be scrutinized doubly.

fingolfin commented 3 months ago

Oh, and of course if there are similar *_mat_one methods for the other rings, e.g. fmpz_mat_one, we could also switch the code to use those, I didn't check for that.

thofma commented 3 months ago

Thanks. About the other _one methods, those can be added at some point I guess.