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

Undocumented argument for NewFilter/DeclareFilter #4408

Closed
fingolfin opened this issue Apr 16, 2021 · 2 comments · Fixed by #4444
Closed

Undocumented argument for NewFilter/DeclareFilter #4408

fingolfin opened this issue Apr 16, 2021 · 2 comments · Fixed by #4444
Labels
topic: documentation Issues and PRs related to documentation

Comments

@fingolfin
Copy link
Member

The manual gives the signatures NewFilter( name[, rank] ) and DeclareFilter( name[, rank] ). But looking at the code, there is a third optional argument: NewFilter( <name>[, <implied>][, <rank>] ) and likewise for DeclareFilter.

As far as I can tell, nothing uses this argument for NewFilter, but some callers for DeclareFilter do; mostly in HPC-GAP code, but also in a few other files, and most importantly, in packages.

So I guess we should simply document this extra argument.

@ThomasBreuer
Copy link
Contributor

Implications between filters can cause problems when one manually resets a filter for a given object: Shall the implied filters also be reset?
As far as I remember, NewFilter/DeclareFilter has been introduced in order to model filters that can be set and reset (one could call them "switch") in an object, whereas NewCategory/DeclareCategory were intended for filters that should not be reset in an object once they had been set.
The NewFilter/DeclareFilter filters should then not imply other filters, whereas the NewCategory/DeclareCategory filters may imply other filters.
Of course these rules/conventions are not in the documentation.
Once we document implied filters as admissible for NewFilter/DeclareFilter, these rules do not make sense at all.
(But then we should at least document the abovementioned problem with implications somewhere.)

@fingolfin
Copy link
Member Author

We discussed this orally, and decided that this feature has been out and in use for too long; so it seems best to just document it. Perhaps with another stern warning that this makes it even more risky to use ResetFilterObj on such filters, and that one should better use DeclareCategory instead...

ThomasBreuer added a commit to ThomasBreuer/gap that referenced this issue Apr 29, 2021
and of `DeclareFilter`.
The optional argument `<implied>` of the two functions had been available
for a long time, but it was not documented.

Now we document it, as dicussed in issue gap-system#4408.

(resolves issue gap-system#4408)
fingolfin pushed a commit that referenced this issue Apr 30, 2021
and of `DeclareFilter`.
The optional argument `<implied>` of the two functions had been available
for a long time, but it was not documented.

Now we document it, as dicussed in issue #4408.

(resolves issue #4408)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: documentation Issues and PRs related to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants