gap-packages / guava

GAP package guava - computations relative to error-correcting codes
https://gap-packages.github.io/guava
Other
13 stars 7 forks source link

Remove bad implication IsLinearCodeRep => IsCodeDefaultRep #43

Closed fingolfin closed 5 years ago

fingolfin commented 5 years ago

Despite its misleading name, IsLinearCodeRep is not a representation but rather a plain filter. As such, it should not imply a representation, as any object, with an arbitrary representation, may have this filter set.

Besides removing the implication, we also need to adjust code that relied on it before.

In future GAP versions, such implications may become illegal, see https://github.com/gap-system/gap/pull/3006.

BTW, I would also recommend renaming IsLinearCodeRep, replacing the misleading Rep suffix.

codecov[bot] commented 5 years ago

Codecov Report

Merging #43 into master will decrease coverage by 0.61%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #43      +/-   ##
==========================================
- Coverage   48.73%   48.11%   -0.62%     
==========================================
  Files          82       82              
  Lines       19562    20246     +684     
==========================================
+ Hits         9533     9742     +209     
- Misses      10029    10504     +475
Impacted Files Coverage Δ
lib/codegen.gi 54.37% <100%> (-0.03%) :arrow_down:
src/leon/src/orbit.c 86.36% <0%> (-8.38%) :arrow_down:
src/leon/src/factor.c 76.38% <0%> (-3.94%) :arrow_down:
src/leon/src/chbase.c 40.92% <0%> (-2.11%) :arrow_down:
src/leon/src/ptstbref.c 63.38% <0%> (-1.54%) :arrow_down:
src/leon/src/partn.c 16.66% <0%> (-1.52%) :arrow_down:
src/leon/src/desauto.c 38.63% <0%> (-1.49%) :arrow_down:
src/leon/src/wtdist.c 60.42% <0%> (-1.28%) :arrow_down:
src/leon/src/readgrp.c 24.8% <0%> (-1.12%) :arrow_down:
src/leon/src/cdesauto.c 88.34% <0%> (-1.12%) :arrow_down:
... and 27 more
fingolfin commented 5 years ago

@osj1961 Does this look reasonable to you?

fingolfin commented 5 years ago

@osj1961 ping?

fingolfin commented 5 years ago

@osj1961 Joe, is there any reason not to merge this?

osj1961 commented 5 years ago

Sorry, I wanted to really understand what this PR was about. I still don't, but as the checks all pass I'm merging.