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

introduce IsMatrixOrMatrixObj #4518

Merged
merged 4 commits into from
May 31, 2021

Conversation

ThomasBreuer
Copy link
Contributor

as sketched in issue #4503.

In this first step, I have changed the documentation of the defining filters and replaced IsMatrixObj by IsMatrixOrMatrixObj in most places.

(In a next step, we can now change the method installations mentioned in issue #4503.)

as sketched in issue gap-system#4503.
In this first step, I have changed the documentation of the defining filters
and replaced `IsMatrixObj` by `IsMatrixOrMatrixObj` in most places.

(In a next step, we can now change the method installations mentioned in
issue gap-system#4503.)
@ThomasBreuer ThomasBreuer added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels May 27, 2021
@fingolfin
Copy link
Member

testlibgap fails -- we need to either change tst/testlibgap/api.c:78 or else adjust GAP_IsMatrixObj to check IsMatrixOrMatrixObj instead of IsMatrixObj...

lib/matobj1.gd Outdated
## <Filt Name="IsMatrixOrMatrixObj" Arg='obj' Type="category"/>
##
## <Description>
## This filter is the <Q>common roof</Q> for <Ref Filt="IsMatrix"/> and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if "common roof" will be understood ? Perhaps we could borrow from the IsListOrCollection documentation, which starts like this:

Several functions are defined for both lists and collections, [...]
IsListOrCollection is a supercategory of IsList and IsCollection (that is, all lists and collections lie in this category)

... also in `libgap-api`.
If we take the filters `IsMatrixOrMatrixObj`, `IsMatrix`, `IsMatrixObj`
seriously then their meaning must be reflected also in the libgap interface.
(Currently `IsMatrix` is not exposed in the interface,
perhaps it should be added now?)

Currently there is no positive test for `IsMatrixObj` in `tst/testlibgap/api.c`.
as suggested by @fingolfin.
Note that this formulation still appears in `lib/word.gd`, see `?IsWord`.
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are obvious things to do next (like changing a bunch of methods back to IsMatrixObj and simplifying their code), but I don't think we need to wait for this. From my POV this can be merged right away.

@ThomasBreuer if you agree, let's do it, and let's continue work afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants