Hecke.jl icon indicating copy to clipboard operation
Hecke.jl copied to clipboard

Add `residue_ring(::ZZRing, ::ZZIdl)`

Open lgoettgens opened this issue 11 months ago • 3 comments

This change makes the code in https://github.com/oscar-system/Oscar.jl/issues/2915#issuecomment-1801794028 run. And I find it nice to have for consistency (see https://github.com/oscar-system/Oscar.jl/issues/2915#issuecomment-1801794028).

lgoettgens avatar Mar 04 '24 15:03 lgoettgens

Codecov Report

Merging #1427 (190d946) into master (cc0a2d8) will decrease coverage by 0.14%. The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1427      +/-   ##
==========================================
- Coverage   70.85%   70.71%   -0.14%     
==========================================
  Files         351      351              
  Lines      111981   111982       +1     
==========================================
- Hits        79340    79185     -155     
- Misses      32641    32797     +156     
Files Coverage Δ
src/NumField/QQ.jl 59.42% <100.00%> (+1.02%) :arrow_up:

... and 33 files with indirect coverage changes

codecov[bot] avatar Mar 04 '24 16:03 codecov[bot]

Should we add the following tests?

@test_throws residue_field(ZZ, -5)   # Should this throw or give GF(5)???
@test residue_field(ZZ, ideal(ZZ, 5))[1] == GF(5)
@test residue_field(ZZ, ideal(ZZ, -5))[1] == GF(5)
@test_throws residue_field(ZZ, ideal(ZZ, 4))
@test_throws residue_field(ZZ, ideal(ZZ, -4))
@test_throws residue_field(ZZ, ideal(ZZ, 0))

JohnAAbbott avatar Mar 06 '24 14:03 JohnAAbbott

I think all of this is not really relevant for residue_field(::ZZRing, ::ZZIdl) as it just calls residue_field(::ZZRing, ::ZZRingElem) and all of this should be already tested there.

lgoettgens avatar Mar 06 '24 14:03 lgoettgens

Bump @thofma

lgoettgens avatar Mar 15 '24 10:03 lgoettgens