Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove bad implication IsLinearCodeRep => IsCodeDefaultRep #43

Merged
merged 1 commit into from
Jun 12, 2019
Merged

Remove bad implication IsLinearCodeRep => IsCodeDefaultRep #43

merged 1 commit into from
Jun 12, 2019

Conversation

fingolfin
Copy link
Member

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 gap-system/gap#3006.

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

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.
@codecov
Copy link

codecov bot commented Mar 24, 2019

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%) ⬇️
src/leon/src/orbit.c 86.36% <0%> (-8.38%) ⬇️
src/leon/src/factor.c 76.38% <0%> (-3.94%) ⬇️
src/leon/src/chbase.c 40.92% <0%> (-2.11%) ⬇️
src/leon/src/ptstbref.c 63.38% <0%> (-1.54%) ⬇️
src/leon/src/partn.c 16.66% <0%> (-1.52%) ⬇️
src/leon/src/desauto.c 38.63% <0%> (-1.49%) ⬇️
src/leon/src/wtdist.c 60.42% <0%> (-1.28%) ⬇️
src/leon/src/readgrp.c 24.8% <0%> (-1.12%) ⬇️
src/leon/src/cdesauto.c 88.34% <0%> (-1.12%) ⬇️
... and 27 more

@fingolfin
Copy link
Member Author

@osj1961 Does this look reasonable to you?

@fingolfin
Copy link
Member Author

@osj1961 ping?

@fingolfin
Copy link
Member Author

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

@osj1961 osj1961 merged commit 0383a6e into gap-packages:master Jun 12, 2019
@osj1961
Copy link
Collaborator

osj1961 commented Jun 12, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants