Add ones for NCRing
Also simplify zeros implementation and add some tests
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 88.13%. Comparing base (
475eafc) to head (b03d06d). Report is 184 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #1886 +/- ##
==========================================
- Coverage 88.13% 88.13% -0.01%
==========================================
Files 120 120
Lines 30286 30281 -5
==========================================
- Hits 26694 26689 -5
Misses 3592 3592
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Hrm, I am torn on this.
We "usually don't want that" for our own matrices, but it is what ones and zeros does in Julia, e.g.
julia> M = ones(BigInt, 3, 3)
3×3 Matrix{BigInt}:
1 1 1
1 1 1
1 1 1
julia> M[1,1] === M[1,2]
true
Of course this is not practical for actual use within OSCAR, but that's not what this is about.
Perhaps this is an argument to turn both of these functions into just error like error("please use zero_matrix instead") or so.
I don't consider it a problem that all elements are identical.
We just had someone on slack that was trapped by the julia behavior. Of course this is just a vote about how misleading the julia function is, not that we should do something else.
We had a discussion related to this during yesterday's triage and basically I now think that while "formally and legally" my new methods are "correct", they are footguns waiting to be triggered, so I now tend to agree that if we add ones it should be a modified duplicate of the existing zeros method, i.e. without aliasing.