-
Notifications
You must be signed in to change notification settings - Fork 160
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
Omit tester filters from ShowImpliedFilters
output
#2243
Omit tester filters from ShowImpliedFilters
output
#2243
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement! Just two minor remarks.
lib/methwhy.g
Outdated
@@ -393,7 +394,7 @@ BIND_GLOBAL("PageSource", function ( fun, nr... ) | |||
f := Filename(List(GAPInfo.RootPaths, Directory), f); | |||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo, r instead of t
lib/methwhy.g
Outdated
@@ -322,20 +319,24 @@ local flags, implied, f, extra_implications, implication, name, diff_reqs, diff_ | |||
implied := SUB_FLAGS(implied, flags); | |||
fi; | |||
|
|||
reduced:= trues -> Filtered( trues, | |||
i -> not ( INFO_FILTERS[i] in FNUM_TPRS | |||
and FLAG1_FILTER( FILTERS[i] ) in trues ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
f2336a2
to
b226edf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.