Omit tester filters from ShowImpliedFilters output
#2243
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fingolfin
requested changes
Mar 5, 2018
Member
fingolfin
left a comment
fingolfin
left a comment
There was a problem hiding this comment.
Nice improvement! Just two minor remarks.
lib/methwhy.g
Outdated
| fi; | ||
| if f = fail and fun in OPERATIONS then | ||
| # for operations we show the location(s) of their operation | ||
| # for operations we show the location(s) of their declararion |
|
|
||
| reduced:= trues -> Filtered( trues, | ||
| i -> not ( INFO_FILTERS[i] in FNUM_TPRS | ||
| and FLAG1_FILTER( FILTERS[i] ) in trues ) ); |
Member
There was a problem hiding this comment.
For HPC-GAP, this requires atomic readonly FILTER_REGION do ... od around the access to INFO_FILTERS and FILTERS, I am afraid. So (modulo typos):
reduced := functon(trues)
atomic readonly FILTER_REGION do
return not ( INFO_FILTERS[i] in FNUM_TPRS
and FLAG1_FILTER( FILTERS[i] ) in trues ) );
od;
end;
Codecov Report
@@ Coverage Diff @@
## master #2243 +/- ##
==========================================
+ Coverage 70.57% 70.57% +<.01%
==========================================
Files 481 481
Lines 253237 253244 +7
==========================================
+ Hits 178728 178735 +7
Misses 74509 74509
|
This is a follow-up to gap-system#2224. As discussed there, those property tester filters are omitted from the output for which the property itself is already shown.
ThomasBreuer
force-pushed
the
TB_ShowImpliedFilters
branch
from
f2336a2 to
b226edf
Compare
fingolfin
approved these changes
Mar 20, 2018
Member
fingolfin
left a comment
fingolfin
left a comment
There was a problem hiding this comment.
Thanks, LGTM!
Off-topic: I just now noticed this PR had been updated, by pure chance -- unfortunately, when new code is pushed into a PR, GitHub does not notify reviewers. So I suggest to add a brief comment, like "updated this PR", so that reviewers may notice the change and update their reviews.
fingolfin
added
the
kind: enhancement
fingolfin
added
the
release notes: added
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow-up to #2224. As discussed there,
those property tester filters are omitted from the output
for which the property itself is already shown.